[clang] 66bcb14 - [SyntaxTree][Synthesis] Fix: `deepCopy` -> `deepCopyExpandingMacros`.
Eduardo Caldas via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 22 02:15:43 PDT 2020
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);
More information about the cfe-commits
mailing list