Author: Eduardo Caldas Date: 2020-09-22T09:15:21Z New Revision: 66bcb14312a08b5d7e1197d23d748b2e23c4d852
URL: https://github.com/llvm/llvm-project/commit/66bcb14312a08b5d7e1197d23d748b2e23c4d852 DIFF: https://github.com/llvm/llvm-project/commit/66bcb14312a08b5d7e1197d23d748b2e23c4d852.diff LOG: [SyntaxTree][Synthesis] Fix: `deepCopy` -> `deepCopyExpandingMacros`. There can be Macros that are tagged with `modifiable`. Thus verifying `canModifyAllDescendants` is not sufficient to avoid macros when deep copying. We think the `TokenBuffer` could inform us whether a `Token` comes from a macro. We'll look into that when we can surface this information easily, for instance in unit tests for `ComputeReplacements`. Differential Revision: https://reviews.llvm.org/D88034 Added: Modified: clang/include/clang/Tooling/Syntax/BuildTree.h clang/lib/Tooling/Syntax/Synthesis.cpp clang/unittests/Tooling/Syntax/SynthesisTest.cpp clang/unittests/Tooling/Syntax/TreeTest.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Tooling/Syntax/BuildTree.h b/clang/include/clang/Tooling/Syntax/BuildTree.h index ab4868747e69..537ade4b7151 100644 --- a/clang/include/clang/Tooling/Syntax/BuildTree.h +++ b/clang/include/clang/Tooling/Syntax/BuildTree.h @@ -45,17 +45,13 @@ createTree(syntax::Arena &A, // Synthesis of Syntax Nodes syntax::EmptyStatement *createEmptyStatement(syntax::Arena &A); -/// Creates a completely independent copy of `N` (a deep copy). +/// Creates a completely independent copy of `N` with its macros expanded. /// /// The copy is: /// * Detached, i.e. `Parent == NextSibling == nullptr` and /// `Role == Detached`. /// * Synthesized, i.e. `Original == false`. -/// -/// `N` might be backed by source code but if any descendants of `N` are -/// unmodifiable returns `nullptr`. -syntax::Node *deepCopy(syntax::Arena &A, const syntax::Node *N); - +syntax::Node *deepCopyExpandingMacros(syntax::Arena &A, const syntax::Node *N); } // namespace syntax } // namespace clang #endif diff --git a/clang/lib/Tooling/Syntax/Synthesis.cpp b/clang/lib/Tooling/Syntax/Synthesis.cpp index 9dc83f966d8c..e197c8d35bde 100644 --- a/clang/lib/Tooling/Syntax/Synthesis.cpp +++ b/clang/lib/Tooling/Syntax/Synthesis.cpp @@ -201,25 +201,11 @@ syntax::Tree *clang::syntax::createTree( return T; } -namespace { -bool canModifyAllDescendants(const syntax::Node *N) { - if (const auto *L = dyn_cast<syntax::Leaf>(N)) - return L->canModify(); - - const auto *T = cast<syntax::Tree>(N); - - if (!T->canModify()) - return false; - for (const auto *Child = T->getFirstChild(); Child; - Child = Child->getNextSibling()) - if (!canModifyAllDescendants(Child)) - return false; - - return true; -} - -syntax::Node *deepCopyImpl(syntax::Arena &A, const syntax::Node *N) { +syntax::Node *clang::syntax::deepCopyExpandingMacros(syntax::Arena &A, + const syntax::Node *N) { if (const auto *L = dyn_cast<syntax::Leaf>(N)) + // `L->getToken()` gives us the expanded token, thus we implicitly expand + // any macros here. return createLeaf(A, L->getToken()->kind(), L->getToken()->text(A.getSourceManager())); @@ -227,18 +213,10 @@ syntax::Node *deepCopyImpl(syntax::Arena &A, const syntax::Node *N) { std::vector<std::pair<syntax::Node *, syntax::NodeRole>> Children; for (const auto *Child = T->getFirstChild(); Child; Child = Child->getNextSibling()) - Children.push_back({deepCopyImpl(A, Child), Child->getRole()}); + Children.push_back({deepCopyExpandingMacros(A, Child), Child->getRole()}); return createTree(A, Children, N->getKind()); } -} // namespace - -syntax::Node *clang::syntax::deepCopy(syntax::Arena &A, const Node *N) { - if (!canModifyAllDescendants(N)) - return nullptr; - - return deepCopyImpl(A, N); -} syntax::EmptyStatement *clang::syntax::createEmptyStatement(syntax::Arena &A) { return cast<EmptyStatement>( diff --git a/clang/unittests/Tooling/Syntax/SynthesisTest.cpp b/clang/unittests/Tooling/Syntax/SynthesisTest.cpp index 2af1fcf8f317..7f67b4e2e203 100644 --- a/clang/unittests/Tooling/Syntax/SynthesisTest.cpp +++ b/clang/unittests/Tooling/Syntax/SynthesisTest.cpp @@ -173,7 +173,7 @@ TEST_P(SynthesisTest, DeepCopy_Synthesized) { {LeafSemiColon, NodeRole::Unknown}}, NodeKind::ContinueStatement); - auto *Copy = deepCopy(*Arena, StatementContinue); + auto *Copy = deepCopyExpandingMacros(*Arena, StatementContinue); EXPECT_TRUE( treeDumpEqual(Copy, StatementContinue->dump(Arena->getSourceManager()))); // FIXME: Test that copy is independent of original, once the Mutations API is @@ -183,7 +183,7 @@ TEST_P(SynthesisTest, DeepCopy_Synthesized) { TEST_P(SynthesisTest, DeepCopy_Original) { auto *OriginalTree = buildTree("int a;", GetParam()); - auto *Copy = deepCopy(*Arena, OriginalTree); + auto *Copy = deepCopyExpandingMacros(*Arena, OriginalTree); EXPECT_TRUE(treeDumpEqual(Copy, R"txt( TranslationUnit Detached synthesized `-SimpleDeclaration synthesized @@ -197,7 +197,7 @@ TranslationUnit Detached synthesized TEST_P(SynthesisTest, DeepCopy_Child) { auto *OriginalTree = buildTree("int a;", GetParam()); - auto *Copy = deepCopy(*Arena, OriginalTree->getFirstChild()); + auto *Copy = deepCopyExpandingMacros(*Arena, OriginalTree->getFirstChild()); EXPECT_TRUE(treeDumpEqual(Copy, R"txt( SimpleDeclaration Detached synthesized |-'int' synthesized @@ -216,8 +216,41 @@ void test() { })cpp", GetParam()); - auto *Copy = deepCopy(*Arena, OriginalTree); - EXPECT_TRUE(Copy == nullptr); + auto *Copy = deepCopyExpandingMacros(*Arena, OriginalTree); + + // The syntax tree stores already expanded Tokens, we can only see whether the + // macro was expanded when computing replacements. The dump does show that + // nodes in the copy are `modifiable`. + EXPECT_TRUE(treeDumpEqual(Copy, R"txt( +TranslationUnit Detached synthesized +`-SimpleDeclaration synthesized + |-'void' synthesized + |-SimpleDeclarator Declarator synthesized + | |-'test' synthesized + | `-ParametersAndQualifiers synthesized + | |-'(' OpenParen synthesized + | `-')' CloseParen synthesized + `-CompoundStatement synthesized + |-'{' OpenParen synthesized + |-IfStatement Statement synthesized + | |-'if' IntroducerKeyword synthesized + | |-'(' synthesized + | |-BinaryOperatorExpression synthesized + | | |-IntegerLiteralExpression LeftHandSide synthesized + | | | `-'1' LiteralToken synthesized + | | |-'+' OperatorToken synthesized + | | `-IntegerLiteralExpression RightHandSide synthesized + | | `-'1' LiteralToken synthesized + | |-')' synthesized + | |-CompoundStatement ThenStatement synthesized + | | |-'{' OpenParen synthesized + | | `-'}' CloseParen synthesized + | |-'else' ElseKeyword synthesized + | `-CompoundStatement ElseStatement synthesized + | |-'{' OpenParen synthesized + | `-'}' CloseParen synthesized + `-'}' CloseParen synthesized + )txt")); } TEST_P(SynthesisTest, Statement_EmptyStatement) { diff --git a/clang/unittests/Tooling/Syntax/TreeTest.cpp b/clang/unittests/Tooling/Syntax/TreeTest.cpp index 2448db36a465..6e777b353b04 100644 --- a/clang/unittests/Tooling/Syntax/TreeTest.cpp +++ b/clang/unittests/Tooling/Syntax/TreeTest.cpp @@ -22,8 +22,8 @@ class TreeTest : public SyntaxTreeTest { std::vector<std::pair<Node *, NodeRole>> ChildrenWithRoles; ChildrenWithRoles.reserve(Children.size()); for (const auto *Child : Children) { - ChildrenWithRoles.push_back( - std::make_pair(deepCopy(*Arena, Child), NodeRole::Unknown)); + ChildrenWithRoles.push_back(std::make_pair( + deepCopyExpandingMacros(*Arena, Child), NodeRole::Unknown)); } return clang::syntax::createTree(*Arena, ChildrenWithRoles, NodeKind::UnknownExpression); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits