eduucaldas updated this revision to Diff 288559. eduucaldas added a comment.
Move `getDeclaratorRange` helpers outside of `TreeBuilder` Add coverage for qualified declarators and init-declarators Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86719/new/ https://reviews.llvm.org/D86719 Files: clang/lib/Tooling/Syntax/BuildTree.cpp clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp =================================================================== --- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp +++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp @@ -3123,6 +3123,35 @@ )txt"})); } +TEST_P(SyntaxTreeTest, OutOfLineMemberFunctionDefinition) { + if (!GetParam().isCXX11OrLater()) { + return; + } + EXPECT_TRUE(treeDumpEqualOnAnnotations( + R"cpp( +struct S { + void f(); +}; +[[void S::f(){}]] +)cpp", + {R"txt( +SimpleDeclaration +|-'void' +|-SimpleDeclarator Declarator +| |-NestedNameSpecifier +| | |-IdentifierNameSpecifier ListElement +| | | `-'S' +| | `-'::' ListDelimiter +| |-'f' +| `-ParametersAndQualifiers +| |-'(' OpenParen +| `-')' CloseParen +`-CompoundStatement + |-'{' OpenParen + `-'}' CloseParen +)txt"})); +} + TEST_P(SyntaxTreeTest, ConversionMemberFunction) { if (!GetParam().isCXX()) { return; @@ -3792,6 +3821,53 @@ )txt")); } +TEST_P(SyntaxTreeTest, InitDeclarator_Brace) { + if (!GetParam().isCXX11OrLater()) { + return; + } + EXPECT_TRUE(treeDumpEqual( + R"cpp( +int a {}; +)cpp", + R"txt( +TranslationUnit Detached +`-SimpleDeclaration + |-'int' + |-SimpleDeclarator Declarator + | |-'a' + | `-UnknownExpression + | `-UnknownExpression + | |-'{' + | `-'}' + `-';' +)txt")); +} + +TEST_P(SyntaxTreeTest, InitDeclarator_Paren) { + if (!GetParam().isCXX()) { + return; + } + EXPECT_TRUE(treeDumpEqualOnAnnotations( + R"cpp( +struct S { + S(int); +}; +[[S s(1);]] +)cpp", + {R"txt( +SimpleDeclaration +|-'S' +|-SimpleDeclarator Declarator +| `-UnknownExpression +| |-'s' +| |-'(' +| |-IntegerLiteralExpression +| | `-'1' LiteralToken +| `-')' +`-';' +)txt"})); +} + TEST_P(SyntaxTreeTest, ArrayDeclarator_Simple) { EXPECT_TRUE(treeDumpEqual( R"cpp( Index: clang/lib/Tooling/Syntax/BuildTree.cpp =================================================================== --- clang/lib/Tooling/Syntax/BuildTree.cpp +++ clang/lib/Tooling/Syntax/BuildTree.cpp @@ -197,18 +197,57 @@ llvm_unreachable("Unknown OverloadedOperatorKind enum"); } +/// Get the start of the qualified name. In the examples below it gives the +/// location of the `^`: +/// `int ^a;` +/// `int ^a::S::f(){}` +static SourceLocation getQualifiedNameStart(NamedDecl *D) { + assert((isa<DeclaratorDecl, TypedefNameDecl>(D)) && + "only DeclaratorDecl and TypedefNameDecl are supported."); + + auto DN = D->getDeclName(); + bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo(); + if (IsAnonymous) + return SourceLocation(); + + if (const auto *DD = dyn_cast<DeclaratorDecl>(D)) { + if (DD->getQualifierLoc()) { + return DD->getQualifierLoc().getBeginLoc(); + } + } + + return D->getLocation(); +} + +/// Gets the range of the initializer inside an init-declarator C++ [dcl.decl]. +/// `int a;` -> range of ``, +/// `int *a = nullptr` -> range of `= nullptr`. +/// `int a{}` -> range of `{}`. +/// `int a()` -> range of `()`. +static SourceRange getInitializerRange(Decl *D) { + if (auto *V = dyn_cast<VarDecl>(D)) { + auto *I = V->getInit(); + // Initializers in range-based-for are not part of the declarator + if (I && !V->isCXXForRangeDecl()) + return I->getSourceRange(); + } + + return SourceRange(); +} + /// Gets the range of declarator as defined by the C++ grammar. E.g. /// `int a;` -> range of `a`, /// `int *a;` -> range of `*a`, /// `int a[10];` -> range of `a[10]`, /// `int a[1][2][3];` -> range of `a[1][2][3]`, /// `int *a = nullptr` -> range of `*a = nullptr`. -/// FIMXE: \p Name must be a source range, e.g. for `operator+`. +/// `int S::f(){}` -> range of `S::f()`. +/// FIXME: \p Name must be a source range, e.g. for `operator+`. static SourceRange getDeclaratorRange(const SourceManager &SM, TypeLoc T, SourceLocation Name, SourceRange Initializer) { SourceLocation Start = GetStartLoc().Visit(T); - SourceLocation End = T.getSourceRange().getEnd(); + SourceLocation End = T.getEndLoc(); assert(End.isValid()); if (Name.isValid()) { if (Start.isInvalid()) @@ -378,11 +417,9 @@ /// Returns true if \p D is the last declarator in a chain and is thus /// reponsible for creating SimpleDeclaration for the whole chain. - template <class T> - bool isResponsibleForCreatingDeclaration(const T *D) const { - static_assert((std::is_base_of<DeclaratorDecl, T>::value || - std::is_base_of<TypedefNameDecl, T>::value), - "only DeclaratorDecl and TypedefNameDecl are supported."); + bool isResponsibleForCreatingDeclaration(const Decl *D) const { + assert((isa<DeclaratorDecl, TypedefNameDecl>(D)) && + "only DeclaratorDecl and TypedefNameDecl are supported."); const Decl *Next = D->getNextDeclInContext(); @@ -390,15 +427,14 @@ if (Next == nullptr) { return true; } - const auto *NextT = dyn_cast<T>(Next); // Next sibling is not the same type, this one is responsible. - if (NextT == nullptr) { + if (D->getKind() != Next->getKind()) { return true; } // Next sibling doesn't begin at the same loc, it must be a different // declaration, so this declarator is responsible. - if (NextT->getBeginLoc() != D->getBeginLoc()) { + if (Next->getBeginLoc() != D->getBeginLoc()) { return true; } @@ -1405,43 +1441,12 @@ } private: - template <class T> SourceLocation getQualifiedNameStart(T *D) { - static_assert((std::is_base_of<DeclaratorDecl, T>::value || - std::is_base_of<TypedefNameDecl, T>::value), - "only DeclaratorDecl and TypedefNameDecl are supported."); - - auto DN = D->getDeclName(); - bool IsAnonymous = DN.isIdentifier() && !DN.getAsIdentifierInfo(); - if (IsAnonymous) - return SourceLocation(); - - if (const auto *DD = dyn_cast<DeclaratorDecl>(D)) { - if (DD->getQualifierLoc()) { - return DD->getQualifierLoc().getBeginLoc(); - } - } - - return D->getLocation(); - } - - SourceRange getInitializerRange(Decl *D) { - if (auto *V = dyn_cast<VarDecl>(D)) { - auto *I = V->getInit(); - // Initializers in range-based-for are not part of the declarator - if (I && !V->isCXXForRangeDecl()) - return I->getSourceRange(); - } - - return SourceRange(); - } - /// Folds SimpleDeclarator node (if present) and in case this is the last /// declarator in the chain it also folds SimpleDeclaration node. template <class T> bool processDeclaratorAndDeclaration(T *D) { - SourceRange Initializer = getInitializerRange(D); - auto Range = getDeclaratorRange(Builder.sourceManager(), - D->getTypeSourceInfo()->getTypeLoc(), - getQualifiedNameStart(D), Initializer); + auto Range = getDeclaratorRange( + Builder.sourceManager(), D->getTypeSourceInfo()->getTypeLoc(), + getQualifiedNameStart(D), getInitializerRange(D)); // There doesn't have to be a declarator (e.g. `void foo(int)` only has // declaration, but no declarator). @@ -1464,10 +1469,8 @@ auto ReturnedType = L.getReturnLoc(); // Build node for the declarator, if any. - auto ReturnDeclaratorRange = - getDeclaratorRange(this->Builder.sourceManager(), ReturnedType, - /*Name=*/SourceLocation(), - /*Initializer=*/SourceLocation()); + auto ReturnDeclaratorRange = SourceRange(GetStartLoc().Visit(ReturnedType), + ReturnedType.getEndLoc()); syntax::SimpleDeclarator *ReturnDeclarator = nullptr; if (ReturnDeclaratorRange.isValid()) { ReturnDeclarator = new (allocator()) syntax::SimpleDeclarator;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits