From fb453dd4b525c74064cb34b66505c728017801dc Mon Sep 17 00:00:00 2001 From: Seth Brenith Date: Tue, 20 Aug 2019 08:03:48 -0700 Subject: [PATCH] [torque] Allow single-param annotations in AnnotationSet Extend the order-independent annotation parsing logic to include the following forms: @foo // bare annotation (already supported) @foo(0x70) // decimal literal @foo(HI) // identifier @foo("hello there") // quoted string This is obviously still pretty far from annotations in other languages, which usually support arbitrary expressions and multiple parameters, but I think it's sufficient to cover a pretty good variety of usages. The existing class-field annotations @if and @ifnot are reimplemented in the new style, meaning they could now appear in any order relative to other annotations on the same field (and can be repeated, though I doubt it would be of much use to anybody). Change-Id: I97b7c0c9a541ca3126b5ae3a2484688b04dda9f4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1754947 Commit-Queue: Seth Brenith Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/master@{#63285} --- src/torque/ast.h | 7 +- src/torque/earley-parser.h | 4 +- src/torque/torque-parser.cc | 134 +++++++++++++++++++++++------------- 3 files changed, 93 insertions(+), 52 deletions(-) diff --git a/src/torque/ast.h b/src/torque/ast.h index 40dd1337e0..5ce25cf13a 100644 --- a/src/torque/ast.h +++ b/src/torque/ast.h @@ -841,10 +841,15 @@ struct ConditionalAnnotation { ConditionalAnnotationType type; }; +struct Annotation { + Identifier* name; + base::Optional param; +}; + struct ClassFieldExpression { NameAndTypeExpression name_and_type; base::Optional index; - base::Optional conditional; + std::vector conditions; bool weak; bool const_qualified; bool generate_verify; diff --git a/src/torque/earley-parser.h b/src/torque/earley-parser.h index d3d0c89c42..9f7ba6a7ae 100644 --- a/src/torque/earley-parser.h +++ b/src/torque/earley-parser.h @@ -56,8 +56,8 @@ enum class ParseResultHolderBase::TypeId { kImplicitParameters, kOptionalImplicitParameters, kNameAndExpression, - kConditionalAnnotation, - kOptionalConditionalAnnotation, + kAnnotation, + kVectorOfAnnotation, kClassFieldExpression, kStructFieldExpression, kStdVectorOfNameAndTypeExpression, diff --git a/src/torque/torque-parser.cc b/src/torque/torque-parser.cc index ef875a4f1a..b65b4f3f89 100644 --- a/src/torque/torque-parser.cc +++ b/src/torque/torque-parser.cc @@ -108,13 +108,12 @@ V8_EXPORT_PRIVATE const ParseResultTypeId ParseResultHolder::id = ParseResultTypeId::kNameAndExpression; template <> -V8_EXPORT_PRIVATE const ParseResultTypeId - ParseResultHolder::id = - ParseResultTypeId::kConditionalAnnotation; +V8_EXPORT_PRIVATE const ParseResultTypeId ParseResultHolder::id = + ParseResultTypeId::kAnnotation; template <> V8_EXPORT_PRIVATE const ParseResultTypeId - ParseResultHolder>::id = - ParseResultTypeId::kOptionalConditionalAnnotation; + ParseResultHolder>::id = + ParseResultTypeId::kVectorOfAnnotation; template <> V8_EXPORT_PRIVATE const ParseResultTypeId ParseResultHolder::id = @@ -661,30 +660,57 @@ base::Optional MakeMethodDeclaration( class AnnotationSet { public: AnnotationSet(ParseResultIterator* iter, - const std::set& allowed) { - auto list = iter->NextAs>(); - for (const Identifier* i : list) { - if (allowed.find(i->value) == allowed.end()) { - Lint("Annotation ", i->value, " is not allowed here").Position(i->pos); - } - if (!set_.insert(i->value).second) { - Lint("Duplicate annotation ", i->value).Position(i->pos); + const std::set& allowed_without_param, + const std::set& allowed_with_param) { + auto list = iter->NextAs>(); + for (const Annotation& a : list) { + if (a.param.has_value()) { + if (allowed_with_param.find(a.name->value) == + allowed_with_param.end()) { + const char* error_message = + allowed_without_param.find(a.name->value) == + allowed_without_param.end() + ? " is not allowed here" + : " cannot have parameter here"; + Lint("Annotation ", a.name->value, error_message) + .Position(a.name->pos); + } + map_[a.name->value].push_back(*a.param); + } else { + if (allowed_without_param.find(a.name->value) == + allowed_without_param.end()) { + const char* error_message = + allowed_with_param.find(a.name->value) == allowed_with_param.end() + ? " is not allowed here" + : " requires a parameter here"; + Lint("Annotation ", a.name->value, error_message) + .Position(a.name->pos); + } + if (!set_.insert(a.name->value).second) { + Lint("Duplicate annotation ", a.name->value).Position(a.name->pos); + } } } } bool Contains(const std::string& s) { return set_.find(s) != set_.end(); } + const std::vector& GetParams(const std::string& s) { + return map_[s]; + } private: std::set set_; + std::map> map_; }; base::Optional MakeClassDeclaration( ParseResultIterator* child_results) { AnnotationSet annotations( - child_results, {"@generatePrint", "@noVerifier", "@abstract", - "@dirtyInstantiatedAbstractClass", - "@hasSameInstanceTypeAsParent", "@generateCppClass"}); + child_results, + {"@generatePrint", "@noVerifier", "@abstract", + "@dirtyInstantiatedAbstractClass", "@hasSameInstanceTypeAsParent", + "@generateCppClass"}, + {}); ClassFlags flags = ClassFlag::kNone; bool generate_print = annotations.Contains("@generatePrint"); if (generate_print) flags |= ClassFlag::kGeneratePrint; @@ -720,15 +746,18 @@ base::Optional MakeClassDeclaration( // Filter to only include fields that should be present based on decoration. std::vector fields; - std::copy_if(fields_raw.begin(), fields_raw.end(), std::back_inserter(fields), - [](const ClassFieldExpression& exp) { - if (!exp.conditional.has_value()) return true; - const ConditionalAnnotation& conditional = *exp.conditional; - return conditional.type == ConditionalAnnotationType::kPositive - ? BuildFlags::GetFlag(conditional.condition, "@if") - : !BuildFlags::GetFlag(conditional.condition, - "@ifnot"); - }); + std::copy_if( + fields_raw.begin(), fields_raw.end(), std::back_inserter(fields), + [](const ClassFieldExpression& exp) { + for (const ConditionalAnnotation& condition : exp.conditions) { + if (condition.type == ConditionalAnnotationType::kPositive + ? !BuildFlags::GetFlag(condition.condition, "@if") + : BuildFlags::GetFlag(condition.condition, "@ifnot")) { + return false; + } + } + return true; + }); Declaration* result = MakeNode( name, flags, std::move(extends), std::move(generates), std::move(methods), @@ -1322,22 +1351,22 @@ base::Optional MakeNameAndExpressionFromExpression( ReportError("Constructor parameters need to be named."); } -base::Optional MakeConditionalAnnotation( - ParseResultIterator* child_results) { - auto type_str = child_results->NextAs()->value; - DCHECK(type_str == "@if" || type_str == "@ifnot"); - ConditionalAnnotationType type = type_str == "@if" - ? ConditionalAnnotationType::kPositive - : ConditionalAnnotationType::kNegative; - auto condition = child_results->NextAs(); - return ParseResult{ConditionalAnnotation{condition, type}}; +base::Optional MakeAnnotation(ParseResultIterator* child_results) { + return ParseResult{ + Annotation{child_results->NextAs(), + child_results->NextAs>()}}; } base::Optional MakeClassField(ParseResultIterator* child_results) { - auto conditional = - child_results->NextAs>(); - AnnotationSet annotations(child_results, {"@noVerifier"}); + AnnotationSet annotations(child_results, {"@noVerifier"}, {"@if", "@ifnot"}); bool generate_verify = !annotations.Contains("@noVerifier"); + std::vector conditions; + for (const std::string& condition : annotations.GetParams("@if")) { + conditions.push_back({condition, ConditionalAnnotationType::kPositive}); + } + for (const std::string& condition : annotations.GetParams("@ifnot")) { + conditions.push_back({condition, ConditionalAnnotationType::kNegative}); + } auto weak = child_results->NextAs(); auto const_qualified = child_results->NextAs(); auto name = child_results->NextAs(); @@ -1345,7 +1374,7 @@ base::Optional MakeClassField(ParseResultIterator* child_results) { auto type = child_results->NextAs(); return ParseResult{ClassFieldExpression{{name, type}, index, - conditional, + std::move(conditions), weak, const_qualified, generate_verify}}; @@ -1474,12 +1503,9 @@ struct TorqueGrammar : Grammar { Symbol name = {Rule({&identifier}, MakeIdentifier)}; // Result: Identifier* - Symbol annotation = { + Symbol annotationName = { Rule({Pattern(MatchAnnotation)}, MakeIdentifierFromMatchedInput)}; - // Result: std::vector - Symbol* annotations = List(&annotation); - // Result: std::string Symbol intrinsicName = { Rule({Pattern(MatchIntrinsicName)}, MakeIdentifierFromMatchedInput)}; @@ -1496,6 +1522,22 @@ struct TorqueGrammar : Grammar { Rule({Pattern(MatchDecimalLiteral)}, YieldMatchedInput), Rule({Pattern(MatchHexLiteral)}, YieldMatchedInput)}; + // Result: std::string + Symbol annotationParameter = {Rule({&identifier}), Rule({&decimalLiteral}), + Rule({&externalString})}; + + // Result: std::string + Symbol annotationParameters = { + Rule({Token("("), &annotationParameter, Token(")")})}; + + // Result: Annotation + Symbol annotation = { + Rule({&annotationName, Optional(&annotationParameters)}, + MakeAnnotation)}; + + // Result: std::vector + Symbol* annotations = List(&annotation); + // Result: TypeList Symbol* typeList = List(&type, Token(",")); @@ -1573,14 +1615,8 @@ struct TorqueGrammar : Grammar { Symbol* optionalArraySpecifier = Optional(Sequence({Token("["), &identifier, Token("]")})); - // Result: ConditionalAnnotation - Symbol conditionalAnnotation = { - Rule({OneOf({"@if", "@ifnot"}), Token("("), &identifier, Token(")")}, - MakeConditionalAnnotation)}; - Symbol classField = { - Rule({Optional(&conditionalAnnotation), - annotations, CheckIf(Token("weak")), CheckIf(Token("const")), &name, + Rule({annotations, CheckIf(Token("weak")), CheckIf(Token("const")), &name, optionalArraySpecifier, Token(":"), &type, Token(";")}, MakeClassField)};