From 9bd8e5f2472f9365096f3f36c71c5699fde03793 Mon Sep 17 00:00:00 2001 From: Daniel Clifford Date: Tue, 30 Jun 2020 17:55:16 +0200 Subject: [PATCH] [torque] Unused implicit parameters can be undefined e.g. the following is now valid Torque code: macro TestA(implicit c: Context)() {} macro TestB(): bool { return TestA(); } This is handy for more flexible usage of generics that may or may not use implicit parameters deep inside their specializations. Note that this change doesn't change the fundamental rigor (or lack thereof) around checking the usage of implicit parameters, which already do not require '_' before their parameter identifier if unused. It just silences errors in cases where a call site doesn't implicitly pass a parameter that ultimately doesn't have a use site and adds meaningful error messages in the case that it does. Bug: v8:7793 Change-Id: I559d06c0864a7e79fe52bee5a9a7af9941889748 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2274127 Commit-Queue: Daniel Clifford Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/master@{#68618} --- src/torque/declarable.cc | 8 +++- src/torque/implementation-visitor.cc | 58 +++++++++++++++++------- src/torque/implementation-visitor.h | 3 +- src/torque/types.cc | 7 +++ src/torque/types.h | 2 + test/unittests/torque/torque-unittest.cc | 49 ++++++++++++++++++++ 6 files changed, 108 insertions(+), 19 deletions(-) diff --git a/src/torque/declarable.cc b/src/torque/declarable.cc index fb5ed15f85..935bddebea 100644 --- a/src/torque/declarable.cc +++ b/src/torque/declarable.cc @@ -95,8 +95,12 @@ std::vector Scope::Lookup(const QualifiedName& name) { base::Optional TypeConstraint::IsViolated(const Type* type) const { if (upper_bound && !type->IsSubtypeOf(*upper_bound)) { - return { - ToString("expected ", *type, " to be a subtype of ", **upper_bound)}; + if (type->IsTopType()) { + return TopType::cast(type)->reason(); + } else { + return { + ToString("expected ", *type, " to be a subtype of ", **upper_bound)}; + } } return base::nullopt; } diff --git a/src/torque/implementation-visitor.cc b/src/torque/implementation-visitor.cc index b0f7013d35..a3af277638 100644 --- a/src/torque/implementation-visitor.cc +++ b/src/torque/implementation-visitor.cc @@ -2391,10 +2391,16 @@ VisitResult ImplementationVisitor::GeneratePointerCall( void ImplementationVisitor::AddCallParameter( Callable* callable, VisitResult parameter, const Type* parameter_type, std::vector* converted_arguments, StackRange* argument_range, - std::vector* constexpr_arguments) { - VisitResult converted = GenerateImplicitConvert(parameter_type, parameter); + std::vector* constexpr_arguments, bool inline_macro) { + VisitResult converted; + if ((converted_arguments->size() < callable->signature().implicit_count) && + parameter.type()->IsTopType()) { + converted = GenerateCopy(parameter); + } else { + converted = GenerateImplicitConvert(parameter_type, parameter); + } converted_arguments->push_back(converted); - if (!callable->ShouldBeInlined()) { + if (!inline_macro) { if (converted.IsOnStack()) { argument_range->Extend(converted.stack_range()); } else { @@ -2444,18 +2450,33 @@ VisitResult ImplementationVisitor::GenerateCall( } } + bool inline_macro = callable->ShouldBeInlined(); std::vector implicit_arguments; for (size_t i = 0; i < callable->signature().implicit_count; ++i) { std::string implicit_name = callable->signature().parameter_names[i]->value; base::Optional*> val = TryLookupLocalValue(implicit_name); - if (!val) { - ReportError("implicit parameter '", implicit_name, - "' required for call to '", callable->ReadableName(), - "' is not defined"); + if (val) { + implicit_arguments.push_back( + GenerateFetchFromLocation((*val)->GetLocationReference(*val))); + } else { + VisitResult unititialized = VisitResult::TopTypeResult( + "implicit parameter '" + implicit_name + + "' is not defined when invoking " + callable->ReadableName() + + " at " + PositionAsString(CurrentSourcePosition::Get()), + callable->signature().parameter_types.types[i]); + implicit_arguments.push_back(unititialized); + } + const Type* type = implicit_arguments.back().type(); + if (const TopType* top_type = TopType::DynamicCast(type)) { + if (!callable->IsMacro() || callable->IsExternal()) { + ReportError( + "unititialized implicit parameters can only be passed to " + "Torque-defined macros: the ", + top_type->reason()); + } + inline_macro = true; } - implicit_arguments.push_back( - GenerateFetchFromLocation((*val)->GetLocationReference(*val))); } std::vector converted_arguments; @@ -2467,7 +2488,7 @@ VisitResult ImplementationVisitor::GenerateCall( AddCallParameter(callable, implicit_arguments[current], callable->signature().parameter_types.types[current], &converted_arguments, &argument_range, - &constexpr_arguments); + &constexpr_arguments, inline_macro); } if (this_reference) { @@ -2476,7 +2497,7 @@ VisitResult ImplementationVisitor::GenerateCall( // By now, the this reference should either be a variable, a temporary or // a Slice. In either case the fetch of the VisitResult should succeed. VisitResult this_value = this_reference->GetVisitResult(); - if (method->ShouldBeInlined()) { + if (inline_macro) { if (!this_value.type()->IsSubtypeOf(method->aggregate_type())) { ReportError("this parameter must be a subtype of ", *method->aggregate_type(), " but it is of type ", @@ -2485,7 +2506,7 @@ VisitResult ImplementationVisitor::GenerateCall( } else { AddCallParameter(callable, this_value, method->aggregate_type(), &converted_arguments, &argument_range, - &constexpr_arguments); + &constexpr_arguments, inline_macro); } ++current; } @@ -2495,7 +2516,7 @@ VisitResult ImplementationVisitor::GenerateCall( ? TypeOracle::GetObjectType() : callable->signature().types()[current++]; AddCallParameter(callable, arg, to_type, &converted_arguments, - &argument_range, &constexpr_arguments); + &argument_range, &constexpr_arguments, inline_macro); } size_t label_count = callable->signature().labels.size(); @@ -2559,7 +2580,7 @@ VisitResult ImplementationVisitor::GenerateCall( } result << "))"; return VisitResult(return_type, result.str()); - } else if (macro->ShouldBeInlined()) { + } else if (inline_macro) { std::vector label_blocks; for (Binding* label : arguments.labels) { label_blocks.push_back(label->block); @@ -2874,8 +2895,13 @@ VisitResult ImplementationVisitor::GenerateImplicitConvert( return scope.Yield(GenerateCopy(source)); } else { std::stringstream s; - s << "cannot use expression of type " << *source.type() - << " as a value of type " << *destination_type; + if (const TopType* top_type = TopType::DynamicCast(source.type())) { + s << "undefined expression of type " << *destination_type << ": the " + << top_type->reason(); + } else { + s << "cannot use expression of type " << *source.type() + << " as a value of type " << *destination_type; + } ReportError(s.str()); } } diff --git a/src/torque/implementation-visitor.h b/src/torque/implementation-visitor.h index 14506d0bf5..2969485b37 100644 --- a/src/torque/implementation-visitor.h +++ b/src/torque/implementation-visitor.h @@ -693,7 +693,8 @@ class ImplementationVisitor { const Type* parameter_type, std::vector* converted_arguments, StackRange* argument_range, - std::vector* constexpr_arguments); + std::vector* constexpr_arguments, + bool inline_macro); VisitResult GenerateCall(Callable* callable, base::Optional this_parameter, diff --git a/src/torque/types.cc b/src/torque/types.cc index a01fd59680..4260e8e4cb 100644 --- a/src/torque/types.cc +++ b/src/torque/types.cc @@ -912,6 +912,13 @@ VisitResult VisitResult::NeverResult() { return result; } +VisitResult VisitResult::TopTypeResult(std::string top_reason, + const Type* from_type) { + VisitResult result; + result.type_ = TypeOracle::GetTopType(std::move(top_reason), from_type); + return result; +} + std::tuple Field::GetFieldSizeInformation() const { auto optional = SizeOf(this->name_and_type.type); if (optional.has_value()) { diff --git a/src/torque/types.h b/src/torque/types.h index 7d84ae6a04..d2e857a261 100644 --- a/src/torque/types.h +++ b/src/torque/types.h @@ -756,6 +756,8 @@ class VisitResult { DCHECK(type->IsConstexpr()); } static VisitResult NeverResult(); + static VisitResult TopTypeResult(std::string top_reason, + const Type* from_type); VisitResult(const Type* type, StackRange stack_range) : type_(type), stack_range_(stack_range) { DCHECK(!type->IsConstexpr()); diff --git a/test/unittests/torque/torque-unittest.cc b/test/unittests/torque/torque-unittest.cc index 71f0497230..742748f4ca 100644 --- a/test/unittests/torque/torque-unittest.cc +++ b/test/unittests/torque/torque-unittest.cc @@ -849,6 +849,55 @@ TEST(Torque, FieldAccessOnNonClassType) { HasSubstr("map")); } +TEST(Torque, UnusedImplicit) { + ExpectSuccessfulCompilation(R"( + @export + macro Test1(implicit c: Smi)(a: Object): Object { return a; } + @export + macro Test2(b: Object) { Test1(b); } + )"); + + ExpectFailingCompilation( + R"( + macro Test1(implicit c: Smi)(_a: Object): Smi { return c; } + @export + macro Test2(b: Smi) { Test1(b); } + )", + HasSubstr("undefined expression of type Smi: the implicit " + "parameter 'c' is not defined when invoking Test1 at")); + + ExpectFailingCompilation( + R"( + extern macro Test3(implicit c: Smi)(Object): Smi; + @export + macro Test4(b: Smi) { Test3(b); } + )", + HasSubstr("unititialized implicit parameters can only be passed to " + "Torque-defined macros: the implicit parameter 'c' is not " + "defined when invoking Test3")); + ExpectSuccessfulCompilation( + R"( + macro Test7(implicit c: Smi)(o: T): Smi; + Test7(implicit c: Smi)(o: Smi): Smi { return o; } + @export + macro Test8(b: Smi) { Test7(b); } + )"); + + ExpectFailingCompilation( + R"( + macro Test6(_o: T): T; + macro Test6(implicit c: T)(_o: T): T { + return c; + } + macro Test7(o: T): Smi; + Test7(o: Smi): Smi { return Test6(o); } + @export + macro Test8(b: Smi) { Test7(b); } + )", + HasSubstr("\nambiguous callable : \n Test6(Smi)\ncandidates are:\n " + "Test6(Smi): Smi\n Test6(implicit Smi)(Smi): Smi")); +} + } // namespace torque } // namespace internal } // namespace v8