[PATCH] D88034: [SyntaxTree][Synthesis] Implement `deepCopyExpandingMacros`

2020-09-22 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 293363.
eduucaldas added a comment.

.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88034/new/

https://reviews.llvm.org/D88034

Files:
  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

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -22,8 +22,8 @@
 std::vector> 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);
Index: clang/unittests/Tooling/Syntax/SynthesisTest.cpp
===
--- clang/unittests/Tooling/Syntax/SynthesisTest.cpp
+++ clang/unittests/Tooling/Syntax/SynthesisTest.cpp
@@ -173,7 +173,7 @@
 {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_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 @@
 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 @@
 })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) {
Index: clang/lib/Tooling/Syntax/Synthesis.cpp
===
--- clang/lib/Tooling/Syntax/Synthesis.cpp
+++ clang/lib/Tooling/Syntax/Synthesis.cpp
@@ -201,25 +201,11 @@
   return T;
 }
 
-namespace {
-bool canModifyAllDescendants(const syntax::Node *N) {
-  if (const auto *L = dyn_cast(N))
-return L->canModify();
-
-  const auto *T = cast(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 , const syntax::Node *N) {
+syntax::Node *clang::syntax::deepCopyExpandingMacros(syntax::Arena ,
+

[PATCH] D88034: [SyntaxTree][Synthesis] Implement `deepCopyExpandingMacros`

2020-09-22 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 293361.
eduucaldas added a comment.

Remove buggy `deepCopy`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88034/new/

https://reviews.llvm.org/D88034

Files:
  clang/include/clang/Tooling/Syntax/BuildTree.h
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/unittests/Tooling/Syntax/SynthesisTest.cpp

Index: clang/unittests/Tooling/Syntax/SynthesisTest.cpp
===
--- clang/unittests/Tooling/Syntax/SynthesisTest.cpp
+++ clang/unittests/Tooling/Syntax/SynthesisTest.cpp
@@ -217,7 +217,53 @@
  GetParam());
 
   auto *Copy = deepCopy(*Arena, OriginalTree);
-  EXPECT_TRUE(Copy == nullptr);
+  EXPECT_EQ(Copy, nullptr);
+}
+
+TEST_P(SynthesisTest, DeepCopy_MacroExpanding) {
+  auto *OriginalTree = buildTree(R"cpp(
+#define HALF_IF if (1+
+#define HALF_IF_2 1) {}
+void test() {
+  HALF_IF HALF_IF_2 else {}
+})cpp",
+ GetParam());
+
+  auto *Copy = deepCopyExpandingMacros(*Arena, OriginalTree);
+
+  // The syntax tree stores expanded Tokens already, as a result we can only see
+  // the macro expansion when computing replacements. The dump does show that
+  // nodes are `modifiable` now.
+  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) {
Index: clang/lib/Tooling/Syntax/Synthesis.cpp
===
--- clang/lib/Tooling/Syntax/Synthesis.cpp
+++ clang/lib/Tooling/Syntax/Synthesis.cpp
@@ -201,25 +201,11 @@
   return T;
 }
 
-namespace {
-bool canModifyAllDescendants(const syntax::Node *N) {
-  if (const auto *L = dyn_cast(N))
-return L->canModify();
-
-  const auto *T = cast(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 , const syntax::Node *N) {
+syntax::Node *clang::syntax::deepCopyExpandingMacros(syntax::Arena ,
+ const syntax::Node *N) {
   if (const auto *L = dyn_cast(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 @@
   std::vector> 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 , const Node *N) {
-  if (!canModifyAllDescendants(N))
-return nullptr;
-
-  return deepCopyImpl(A, N);
-}
 
 syntax::EmptyStatement *clang::syntax::createEmptyStatement(syntax::Arena ) {
   return cast(
Index: clang/include/clang/Tooling/Syntax/BuildTree.h
===
--- clang/include/clang/Tooling/Syntax/BuildTree.h
+++ clang/include/clang/Tooling/Syntax/BuildTree.h
@@ -45,17 +45,13 @@
 // Synthesis of Syntax Nodes
 syntax::EmptyStatement *createEmptyStatement(syntax::Arena );
 
-/// 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 

[PATCH] D88034: [SyntaxTree][Synthesis] Implement `deepCopyExpandingMacros`

2020-09-21 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
eduucaldas requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88034

Files:
  clang/include/clang/Tooling/Syntax/BuildTree.h
  clang/lib/Tooling/Syntax/Synthesis.cpp
  clang/unittests/Tooling/Syntax/SynthesisTest.cpp

Index: clang/unittests/Tooling/Syntax/SynthesisTest.cpp
===
--- clang/unittests/Tooling/Syntax/SynthesisTest.cpp
+++ clang/unittests/Tooling/Syntax/SynthesisTest.cpp
@@ -217,7 +217,53 @@
  GetParam());
 
   auto *Copy = deepCopy(*Arena, OriginalTree);
-  EXPECT_TRUE(Copy == nullptr);
+  EXPECT_EQ(Copy, nullptr);
+}
+
+TEST_P(SynthesisTest, DeepCopy_MacroExpanding) {
+  auto *OriginalTree = buildTree(R"cpp(
+#define HALF_IF if (1+
+#define HALF_IF_2 1) {}
+void test() {
+  HALF_IF HALF_IF_2 else {}
+})cpp",
+ GetParam());
+
+  auto *Copy = deepCopyExpandingMacros(*Arena, OriginalTree);
+
+  // The syntax tree stores expanded Tokens already, as a result we can only see
+  // the macro expansion when computing replacements. The dump does show that
+  // nodes are `modifiable` now.
+  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) {
Index: clang/lib/Tooling/Syntax/Synthesis.cpp
===
--- clang/lib/Tooling/Syntax/Synthesis.cpp
+++ clang/lib/Tooling/Syntax/Synthesis.cpp
@@ -218,8 +218,13 @@
   return true;
 }
 
-syntax::Node *deepCopyImpl(syntax::Arena , const syntax::Node *N) {
+} // namespace
+
+syntax::Node *clang::syntax::deepCopyExpandingMacros(syntax::Arena ,
+ const syntax::Node *N) {
   if (const auto *L = dyn_cast(N))
+// For original nodes `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,17 +232,16 @@
   std::vector> 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 , const Node *N) {
   if (!canModifyAllDescendants(N))
 return nullptr;
 
-  return deepCopyImpl(A, N);
+  return deepCopyExpandingMacros(A, N);
 }
 
 syntax::EmptyStatement *clang::syntax::createEmptyStatement(syntax::Arena ) {
Index: clang/include/clang/Tooling/Syntax/BuildTree.h
===
--- clang/include/clang/Tooling/Syntax/BuildTree.h
+++ clang/include/clang/Tooling/Syntax/BuildTree.h
@@ -56,6 +56,16 @@
 /// unmodifiable returns `nullptr`.
 syntax::Node *deepCopy(syntax::Arena , const syntax::Node *N);
 
+/// Creates a completely independent copy of `N` with its macros expanded.
+///
+/// Like `deepCopy` the copy is:
+/// * Detached, i.e. `Parent == NextSibling == nullptr` and
+/// `Role == Detached`.
+/// * Synthesized, i.e. `Original == false`.
+///
+/// However, `N` might have descendants coming from macros, in this case those
+/// macros are expanded.
+syntax::Node *deepCopyExpandingMacros(syntax::Arena , const syntax::Node *N);
 } // namespace syntax
 } // namespace clang
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits