Remove and update some outdated TODO(mstarzinger).

R=clemensb@chromium.org

Change-Id: Ibd6790a222590fd4dce9f918219a19f01c2e1e0f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1960293
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65439}
This commit is contained in:
Michael Starzinger 2019-12-11 13:02:45 +01:00 committed by Commit Bot
parent e99f6ffef3
commit b577c1fe95
30 changed files with 47 additions and 63 deletions

View File

@ -285,8 +285,6 @@ def _CheckHeadersHaveIncludeGuards(input_api, output_api):
return []
# TODO(mstarzinger): Similar checking should be made available as part of
# tools/presubmit.py (note that tools/check-inline-includes.sh exists).
def _CheckNoInlineHeaderIncludesInNormalHeaders(input_api, output_api):
"""Attempts to prevent inclusion of inline headers into normal header
files. This tries to establish a layering where inline headers can be

View File

@ -361,7 +361,7 @@ MaybeHandle<Object> AsmJs::InstantiateAsmWasm(Isolate* isolate,
Handle<WasmModuleObject> module =
wasm_engine->FinalizeTranslatedAsmJs(isolate, wasm_data, script);
// TODO(mstarzinger): The position currently points to the module definition
// TODO(asmjs): The position currently points to the module definition
// but should instead point to the instantiation site (more intuitive).
int position = shared->StartPosition();

View File

@ -240,7 +240,7 @@ void AsmJsParser::AddGlobalImport(Vector<const char> name, AsmType* type,
ValueType vtype, bool mutable_variable,
VarInfo* info) {
// Allocate a separate variable for the import.
// TODO(mstarzinger): Consider using the imported global directly instead of
// TODO(asmjs): Consider using the imported global directly instead of
// allocating a separate global variable for immutable (i.e. const) imports.
DeclareGlobal(info, mutable_variable, type, vtype);
@ -2432,7 +2432,7 @@ void AsmJsParser::ValidateHeapAccess() {
uint32_t offset;
if (CheckForUnsigned(&offset)) {
// TODO(bradnelson): Check more things.
// TODO(mstarzinger): Clarify and explain where this limit is coming from,
// TODO(asmjs): Clarify and explain where this limit is coming from,
// as it is not mandated by the spec directly.
if (offset > 0x7FFFFFFF ||
static_cast<uint64_t>(offset) * static_cast<uint64_t>(size) >

View File

@ -69,7 +69,7 @@ class CodeDesc {
int reloc_size = 0;
// Unwinding information.
// TODO(jgruber,mstarzinger): Pack this into the inlined metadata section.
// TODO(jgruber): Pack this into the inlined metadata section.
byte* unwinding_info = nullptr;
int unwinding_info_size = 0;

View File

@ -809,7 +809,7 @@ class V8_EXPORT_PRIVATE Instruction final {
DCHECK(output_count == 0 || outputs != nullptr);
DCHECK(input_count == 0 || inputs != nullptr);
DCHECK(temp_count == 0 || temps != nullptr);
// TODO(jarin/mstarzinger): Handle this gracefully. See crbug.com/582702.
// TODO(turbofan): Handle this gracefully. See crbug.com/582702.
CHECK(InputCountField::is_valid(input_count));
size_t total_extra_ops = output_count + input_count + temp_count;

View File

@ -2103,8 +2103,6 @@ void BytecodeGraphBuilder::VisitCreateArrayLiteral() {
// data to converge. So, we disable allocation site mementos in optimized
// code. We can revisit this when we have data to the contrary.
literal_flags |= ArrayLiteral::kDisableMementos;
// TODO(mstarzinger): Thread through number of elements. The below number is
// only an estimate and does not match {ArrayLiteral::values::length}.
int number_of_elements =
array_boilerplate_description.constants_elements_length();
Node* literal = NewNode(javascript()->CreateLiteralArray(
@ -2134,8 +2132,6 @@ void BytecodeGraphBuilder::VisitCreateObjectLiteral() {
int bytecode_flags = bytecode_iterator().GetFlagOperand(2);
int literal_flags =
interpreter::CreateObjectLiteralFlags::FlagsBits::decode(bytecode_flags);
// TODO(mstarzinger): Thread through number of properties. The below number is
// only an estimate and does not match {ObjectLiteral::properties_count}.
int number_of_properties = constant_properties.size();
Node* literal = NewNode(javascript()->CreateLiteralObject(
constant_properties.object(), pair, literal_flags, number_of_properties));
@ -3640,7 +3636,6 @@ void BytecodeGraphBuilder::MergeIntoSuccessorEnvironment(int target_offset) {
// Append merge nodes to the environment. We may merge here with another
// environment. So add a place holder for merge nodes. We may add redundant
// but will be eliminated in a later pass.
// TODO(mstarzinger): Be smarter about this!
NewMerge();
merge_environment = environment();
} else {

View File

@ -206,7 +206,7 @@ void ControlEquivalence::DFSPop(DFSStack& stack, Node* node) {
void ControlEquivalence::BracketListDelete(BracketList& blist, Node* to,
DFSDirection direction) {
// TODO(mstarzinger): Optimize this to avoid linear search.
// TODO(turbofan): Optimize this to avoid linear search.
for (BracketList::iterator i = blist.begin(); i != blist.end(); /*nop*/) {
if (i->to == to && i->direction != direction) {
TRACE(" BList erased: {%d->%d}\n", i->from->id(), i->to->id());

View File

@ -161,7 +161,7 @@ Reduction JSCreateLowering::ReduceJSCreateArguments(Node* node) {
if (outer_state->opcode() != IrOpcode::kFrameState) {
switch (type) {
case CreateArgumentsType::kMappedArguments: {
// TODO(mstarzinger): Duplicate parameters are not handled yet.
// TODO(turbofan): Duplicate parameters are not handled yet.
if (shared.has_duplicate_parameters()) return NoChange();
Node* const callee = NodeProperties::GetValueInput(node, 0);
Node* const context = NodeProperties::GetContextInput(node);
@ -263,7 +263,7 @@ Reduction JSCreateLowering::ReduceJSCreateArguments(Node* node) {
Node* const callee = NodeProperties::GetValueInput(node, 0);
Node* const context = NodeProperties::GetContextInput(node);
Node* effect = NodeProperties::GetEffectInput(node);
// TODO(mstarzinger): Duplicate parameters are not handled yet.
// TODO(turbofan): Duplicate parameters are not handled yet.
if (shared.has_duplicate_parameters()) return NoChange();
// Choose the correct frame state and frame state info depending on
// whether there conceptually is an arguments adaptor frame in the call

View File

@ -268,7 +268,6 @@ Node* JSInliner::CreateArtificialFrameState(Node* node, Node* outer_frame_state,
namespace {
// TODO(mstarzinger,verwaest): Move this predicate onto SharedFunctionInfo?
bool NeedsImplicitReceiver(SharedFunctionInfoRef shared_info) {
DisallowHeapAllocation no_gc;
return !shared_info.construct_as_builtin() &&

View File

@ -754,9 +754,8 @@ class SpecialRPONumberer : public ZoneObject {
return block->loop_number() >= 0;
}
// TODO(mstarzinger): We only need this special sentinel because some tests
// use the schedule's end block in actual control flow (e.g. with end having
// successors). Once this has been cleaned up we can use the end block here.
// We only need this special sentinel because some tests use the schedule's
// end block in actual control flow (e.g. with end having successors).
BasicBlock* BeyondEndSentinel() {
if (beyond_end_ == nullptr) {
BasicBlock::Id id = BasicBlock::Id::FromInt(-1);
@ -1757,7 +1756,7 @@ void Scheduler::FuseFloatingControl(BasicBlock* block, Node* node) {
// Iterate on phase 2: Compute special RPO and dominator tree.
special_rpo_->UpdateSpecialRPO(block, schedule_->block(node));
// TODO(mstarzinger): Currently "iterate on" means "re-run". Fix that.
// TODO(turbofan): Currently "iterate on" means "re-run". Fix that.
for (BasicBlock* b = block->rpo_next(); b != nullptr; b = b->rpo_next()) {
b->set_dominator_depth(-1);
b->set_dominator(nullptr);
@ -1765,7 +1764,7 @@ void Scheduler::FuseFloatingControl(BasicBlock* block, Node* node) {
PropagateImmediateDominators(block->rpo_next());
// Iterate on phase 4: Schedule nodes early.
// TODO(mstarzinger): The following loop gathering the propagation roots is a
// TODO(turbofan): The following loop gathering the propagation roots is a
// temporary solution and should be merged into the rest of the scheduler as
// soon as the approach settled for all floating loops.
NodeVector propagation_roots(control_flow_builder_->control_);
@ -1787,7 +1786,7 @@ void Scheduler::FuseFloatingControl(BasicBlock* block, Node* node) {
schedule_early_visitor.Run(&propagation_roots);
// Move previously planned nodes.
// TODO(mstarzinger): Improve that by supporting bulk moves.
// TODO(turbofan): Improve that by supporting bulk moves.
scheduled_nodes_.resize(schedule_->BasicBlockCount());
MovePlannedNodes(block, schedule_->block(node));

View File

@ -18,7 +18,7 @@
#include "src/base/platform/platform.h"
#include "src/objects/feedback-vector.h"
// TODO(mstarzinger): There is one more include to remove in order to no longer
// TODO(gc): There is one more include to remove in order to no longer
// leak heap internals to users of this interface!
#include "src/execution/isolate-data.h"
#include "src/execution/isolate.h"

View File

@ -1644,7 +1644,7 @@ void BytecodeGenerator::BuildTryCatch(
// Preserve the context in a dedicated register, so that it can be restored
// when the handler is entered by the stack-unwinding machinery.
// TODO(mstarzinger): Be smarter about register allocation.
// TODO(ignition): Be smarter about register allocation.
Register context = register_allocator()->NewRegister();
builder()->MoveRegister(Register::current_context(), context);
@ -1695,7 +1695,7 @@ void BytecodeGenerator::BuildTryFinally(
// Preserve the context in a dedicated register, so that it can be restored
// when the handler is entered by the stack-unwinding machinery.
// TODO(mstarzinger): Be smarter about register allocation.
// TODO(ignition): Be smarter about register allocation.
Register context = register_allocator()->NewRegister();
builder()->MoveRegister(Register::current_context(), context);

View File

@ -600,8 +600,6 @@ inline void DisassembleCodeRange(Isolate* isolate, std::ostream& os, Code code,
Address begin, size_t size,
Address current_pc) {
Address end = begin + size;
// TODO(mstarzinger): Refactor CodeReference to avoid the
// unhandlified->handlified transition.
AllowHandleAllocation allow_handles;
DisallowHeapAllocation no_gc;
HandleScope handle_scope(isolate);

View File

@ -474,7 +474,6 @@ class JSObject : public TorqueGeneratedJSObject<JSObject, JSReceiver> {
GetPropertyAttributesWithFailedAccessCheck(LookupIterator* it);
// Defines an AccessorPair property on the given object.
// TODO(mstarzinger): Rename to SetAccessor().
V8_EXPORT_PRIVATE static MaybeHandle<Object> DefineAccessor(
Handle<JSObject> object, Handle<Name> name, Handle<Object> getter,
Handle<Object> setter, PropertyAttributes attributes);

View File

@ -123,7 +123,7 @@ RUNTIME_FUNCTION(Runtime_WasmThrowCreate) {
isolate->set_context(GetNativeContextFromWasmInstanceOnStackTop(isolate));
CONVERT_ARG_CHECKED(WasmExceptionTag, tag_raw, 0);
CONVERT_SMI_ARG_CHECKED(size, 1);
// TODO(mstarzinger): Manually box because parameters are not visited yet.
// TODO(wasm): Manually box because parameters are not visited yet.
Handle<Object> tag(tag_raw, isolate);
Handle<Object> exception = isolate->factory()->NewWasmRuntimeError(
MessageTemplate::kWasmExceptionError);
@ -148,7 +148,7 @@ RUNTIME_FUNCTION(Runtime_WasmExceptionGetTag) {
DCHECK(isolate->context().is_null());
isolate->set_context(GetNativeContextFromWasmInstanceOnStackTop(isolate));
CONVERT_ARG_CHECKED(Object, except_obj_raw, 0);
// TODO(mstarzinger): Manually box because parameters are not visited yet.
// TODO(wasm): Manually box because parameters are not visited yet.
Handle<Object> except_obj(except_obj_raw, isolate);
if (!except_obj->IsWasmExceptionPackage(isolate)) {
return ReadOnlyRoots(isolate).undefined_value();
@ -165,7 +165,7 @@ RUNTIME_FUNCTION(Runtime_WasmExceptionGetValues) {
DCHECK(isolate->context().is_null());
isolate->set_context(GetNativeContextFromWasmInstanceOnStackTop(isolate));
CONVERT_ARG_CHECKED(Object, except_obj_raw, 0);
// TODO(mstarzinger): Manually box because parameters are not visited yet.
// TODO(wasm): Manually box because parameters are not visited yet.
Handle<Object> except_obj(except_obj_raw, isolate);
if (!except_obj->IsWasmExceptionPackage(isolate)) {
return ReadOnlyRoots(isolate).undefined_value();
@ -477,7 +477,7 @@ RUNTIME_FUNCTION(Runtime_WasmFunctionTableSet) {
CONVERT_UINT32_ARG_CHECKED(table_index, 1);
CONVERT_UINT32_ARG_CHECKED(entry_index, 2);
CONVERT_ARG_CHECKED(Object, element_raw, 3);
// TODO(mstarzinger): Manually box because parameters are not visited yet.
// TODO(wasm): Manually box because parameters are not visited yet.
Handle<Object> element(element_raw, isolate);
DCHECK_LT(table_index, instance->tables().length());
auto table = handle(
@ -536,7 +536,7 @@ RUNTIME_FUNCTION(Runtime_WasmTableGrow) {
Handle<WasmInstanceObject>(GetWasmInstanceOnStackTop(isolate), isolate);
CONVERT_UINT32_ARG_CHECKED(table_index, 0);
CONVERT_ARG_CHECKED(Object, value_raw, 1);
// TODO(mstarzinger): Manually box because parameters are not visited yet.
// TODO(wasm): Manually box because parameters are not visited yet.
Handle<Object> value(value_raw, isolate);
CONVERT_UINT32_ARG_CHECKED(delta, 2);
@ -555,7 +555,7 @@ RUNTIME_FUNCTION(Runtime_WasmTableFill) {
CONVERT_UINT32_ARG_CHECKED(table_index, 0);
CONVERT_UINT32_ARG_CHECKED(start, 1);
CONVERT_ARG_CHECKED(Object, value_raw, 2);
// TODO(mstarzinger): Manually box because parameters are not visited yet.
// TODO(wasm): Manually box because parameters are not visited yet.
Handle<Object> value(value_raw, isolate);
CONVERT_UINT32_ARG_CHECKED(count, 3);

View File

@ -1807,8 +1807,8 @@ class WasmFullDecoder : public WasmDecoder<validate> {
auto exception = Pop(0, kWasmExnRef);
const WasmExceptionSig* sig = imm.index.exception->sig;
size_t value_count = sig->parameter_count();
// TODO(mstarzinger): This operand stack mutation is an ugly hack to
// make both type checking here as well as environment merging in the
// TODO(wasm): This operand stack mutation is an ugly hack to make
// both type checking here as well as environment merging in the
// graph builder interface work out of the box. We should introduce
// special handling for both and do minimal/no stack mutation here.
for (size_t i = 0; i < value_count; ++i) Push(sig->GetParam(i));

View File

@ -1142,8 +1142,8 @@ bool InstanceBuilder::ProcessImportedGlobal(Handle<WasmInstanceObject> instance,
// workaround to support legacy asm.js code with broken binding. Note
// that using {NaN} (or Smi::zero()) here is what using the observable
// conversion via {ToPrimitive} would produce as well.
// TODO(mstarzinger): Still observable if Function.prototype.valueOf
// or friends are patched, we might need to check for that as well.
// TODO(wasm): Still observable if Function.prototype.valueOf or friends
// are patched, we might need to check for that as well.
if (value->IsJSFunction()) value = isolate_->factory()->nan_value();
if (value->IsPrimitive() && !value->IsSymbol()) {
if (global.type == kWasmI32) {

View File

@ -374,8 +374,8 @@ StreamingDecoder::DecodeSectionID::Next(StreamingDecoder* streaming) {
// Explicitly check for multiple code sections as module decoder never
// sees the code section and hence cannot track this section.
if (streaming->code_section_processed_) {
// TODO(mstarzinger): This error message (and others in this class) is
// different for non-streaming decoding. Bring them in sync and test.
// TODO(wasm): This error message (and others in this class) is different
// for non-streaming decoding. Bring them in sync and test.
return streaming->Error("code section can only appear once");
}
streaming->code_section_processed_ = true;

View File

@ -204,9 +204,6 @@ class V8_EXPORT_PRIVATE ValueTypes {
(expected == kWasmAnyRef && actual == kWasmFuncRef) ||
(expected == kWasmAnyRef && actual == kWasmExnRef) ||
(expected == kWasmFuncRef && actual == kWasmNullRef) ||
// TODO(mstarzinger): For now we treat "nullref" as a sub-type of
// "exnref", which is correct but might change. See here:
// https://github.com/WebAssembly/exception-handling/issues/55
(expected == kWasmExnRef && actual == kWasmNullRef);
}

View File

@ -1342,9 +1342,9 @@ class ThreadImpl {
// Safety wrapper for values on the operand stack represented as {WasmValue}.
// Most values are stored directly on the stack, only reference values are
// kept in a separate on-heap reference stack to make the GC trace them.
// TODO(mstarzinger): Optimize simple stack operations (like "get_local",
// TODO(wasm): Optimize simple stack operations (like "get_local",
// "set_local", and "tee_local") so that they don't require a handle scope.
// TODO(mstarzinger): Consider optimizing activations that use no reference
// TODO(wasm): Consider optimizing activations that use no reference
// values to avoid allocating the reference stack entirely.
class StackValue {
public:

View File

@ -1345,7 +1345,7 @@ Handle<WasmInstanceObject> WasmInstanceObject::New(
// Insert the new instance into the scripts weak list of instances. This list
// is used for breakpoints affecting all instances belonging to the script.
// TODO(mstarzinger): Allow to reuse holes in the {WeakArrayList} below.
// TODO(wasm): Allow to reuse holes in the {WeakArrayList} below.
if (module_object->script().type() == Script::TYPE_WASM) {
Handle<WeakArrayList> weak_instance_list(
module_object->script().wasm_weak_instance_list(), isolate);
@ -1556,7 +1556,7 @@ void WasmInstanceObject::ImportWasmJSFunctionIntoTable(
if (sig_id >= 0) {
wasm::NativeModule* native_module =
instance->module_object().native_module();
// TODO(mstarzinger): Cache and reuse wrapper code.
// TODO(wasm): Cache and reuse wrapper code.
const wasm::WasmFeatures enabled = native_module->enabled_features();
auto resolved = compiler::ResolveWasmImportCall(callable, sig, enabled);
compiler::WasmImportCallKind kind = resolved.first;
@ -1938,8 +1938,8 @@ Handle<WasmJSFunction> WasmJSFunction::New(Isolate* isolate,
if (sig_size > 0) {
serialized_sig->copy_in(0, sig->all().begin(), sig_size);
}
// TODO(mstarzinger): Think about caching and sharing the JS-to-JS wrappers
// per signature instead of compiling a new one for every instantiation.
// TODO(wasm): Think about caching and sharing the JS-to-JS wrappers per
// signature instead of compiling a new one for every instantiation.
Handle<Code> wrapper_code =
compiler::CompileJSToJSWrapper(isolate, sig).ToHandleChecked();
Handle<WasmJSFunctionData> function_data =

View File

@ -248,7 +248,7 @@ class V8_EXPORT_PRIVATE WasmTableObject : public JSObject {
static void Fill(Isolate* isolate, Handle<WasmTableObject> table,
uint32_t start, Handle<Object> entry, uint32_t count);
// TODO(mstarzinger): Unify these three methods into one.
// TODO(wasm): Unify these three methods into one.
static void UpdateDispatchTables(Isolate* isolate,
Handle<WasmTableObject> table,
int entry_index, wasm::FunctionSig* sig,

View File

@ -66,8 +66,8 @@ class ZoneDeque : public std::deque<T, RecyclingZoneAllocator<T>> {
// A wrapper subclass for std::list to make it easy to construct one
// that uses a zone allocator.
// TODO(mstarzinger): This should be renamed to ZoneList once we got rid of our
// own home-grown ZoneList that actually is a ZoneVector.
// TODO(all): This should be renamed to ZoneList once we got rid of our own
// home-grown ZoneList that actually is a ZoneVector.
template <typename T>
class ZoneLinkedList : public std::list<T, ZoneAllocator<T>> {
public:

View File

@ -36,7 +36,7 @@
##############################################################################
['variant == stress', {
# TODO(jarin/mstarzinger): Functions with eval or debugger now get optimized
# TODO(turbofan): Functions with eval or debugger now get optimized
# with Turbofan, which has issues with the debugger issues.
'debug/debug-evaluate-locals': [FAIL],

View File

@ -91,7 +91,7 @@ function test(use_new, add_first, set__proto__) {
}
%EnsureFeedbackVectorForFunction(test);
// TODO(mstarzinger): This test fails easily if gc happens at the wrong time.
// This test fails easily if gc happens at the wrong time.
gc();
function test_fast_prototype() {

View File

@ -312,7 +312,7 @@
'code-coverage-ad-hoc': [SKIP],
'code-coverage-precise': [SKIP],
# TODO(mstarzinger): Takes too long with TF.
# Takes too long with TF.
'array-sort': [PASS, NO_VARIANTS],
'regress/regress-91008': [PASS, NO_VARIANTS],
'regress/regress-transcendental': [PASS, ['arch == arm64', NO_VARIANTS]],
@ -784,7 +784,7 @@
##############################################################################
['system == windows', {
# TODO(mstarzinger): Too slow with turbo fan.
# Too slow with turbo fan.
'math-floor-of-div': [PASS, ['mode == debug', SKIP]],
'math-floor-of-div-nosudiv': [PASS, ['mode == debug', SKIP]],
'unicodelctest': [PASS, ['mode == debug', SKIP]],

View File

@ -332,7 +332,6 @@
# Test that depends on timer resolution. Fails every now and then
# if we're unlucky enough to get a context switch at a bad time.
# TODO(mstarzinger): Switch off TF on windows due to timeouts.
'js1_5/extensions/regress-363258': [PASS, FAIL, ['system == windows', NO_VARIANTS]],

View File

@ -632,8 +632,8 @@
['arch == arm or arch == mipsel or arch == mips or arch == arm64 or arch == mips64 or arch == mips64el', {
# TODO(mstarzinger): Causes stack overflow on simulators due to eager
# compilation of parenthesized function literals. Needs investigation.
# Causes stack overflow on simulators due to eager compilation of
# parenthesized function literals. Needs investigation.
'language/statements/function/S13.2.1_A1_T1': [SKIP],
# BUG(3251225): Tests that timeout with --noopt.

View File

@ -15,7 +15,7 @@
'elem': [FAIL],
'data': [FAIL],
# TODO(mstarzinger): Roll newest tests into "js-types" repository.
# TODO(wasm): Roll newest tests into "js-types" repository.
'proposals/js-types/exports': [FAIL],
'proposals/js-types/globals': [FAIL],
'proposals/js-types/linking': [FAIL],

View File

@ -907,7 +907,7 @@ class FunctionAnalyzer {
}
DECL_VISIT_EXPR(UnaryOperator) {
// TODO(mstarzinger): We are treating all expressions that look like
// TODO(gcmole): We are treating all expressions that look like
// {&raw_pointer_var} as definitions of {raw_pointer_var}. This should be
// changed to recognize less generic pattern:
//
@ -1039,7 +1039,7 @@ class FunctionAnalyzer {
llvm::dyn_cast_or_null<clang::CXXOperatorCallExpr>(call);
if (opcall != NULL && opcall->isAssignmentOp() &&
IsRawPointerVar(opcall->getArg(0), &var_name)) {
// TODO(mstarzinger): We are treating all assignment operator calls with
// TODO(gcmole): We are treating all assignment operator calls with
// the left hand side looking like {raw_pointer_var} as safe independent
// of the concrete assignment operator implementation. This should be
// changed to be more narrow only if the assignment operator of the base
@ -1416,7 +1416,7 @@ class FunctionAnalyzer {
return out;
}
// TODO(mstarzinger): handle other declarations?
// TODO(gcmole): handle other declarations?
return env;
}