[SyntaxTree][Synthesis] Fix: `deepCopy` -> `deepCopyExpandingMacros`.

[eduucaldas]

Summary

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.

[eduucaldas]

Remove buggy deepCopy

[eduucaldas]

.

clang/include/clang/Tooling/Syntax/BuildTree.h

 syntax::Tree *
 createTree(syntax::Arena &A,
            ArrayRef<std::pair<syntax::Node *, syntax::NodeRole>> Children,
            syntax::NodeKind K);
 
 // 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

clang/lib/Tooling/Syntax/Synthesis.cpp

syntax::Tree *clang::syntax::createTree(
   FactoryImpl::setCanModify(T);
   for (auto ChildIt = Children.rbegin(); ChildIt != Children.rend(); ++ChildIt)
     FactoryImpl::prependChildLowLevel(T, ChildIt->first, ChildIt->second);
 
   T->assertInvariants();
   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()));
 
   const auto *T = cast<syntax::Tree>(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>(
       createTree(A, {{createLeaf(A, tok::semi), NodeRole::Unknown}},
                  NodeKind::EmptyStatement));
 }

clang/unittests/Tooling/Syntax/SynthesisTest.cpp

TEST_P(SynthesisTest, DeepCopy_Synthesized) {
 
   auto *LeafContinue = createLeaf(*Arena, tok::kw_continue);
   auto *LeafSemiColon = createLeaf(*Arena, tok::semi);
   auto *StatementContinue = createTree(*Arena,
                                        {{LeafContinue, NodeRole::LiteralToken},
                                         {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
   // more developed.
 }
 
 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
   |-'int' synthesized
   |-SimpleDeclarator Declarator synthesized
   | `-'a' synthesized
   `-';' synthesized
   )txt"));
 }
 
 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
 |-SimpleDeclarator Declarator synthesized
 | `-'a' synthesized
 `-';' synthesized
   )txt"));
 }
 
 TEST_P(SynthesisTest, DeepCopy_Macro) {
   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 = 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) {
   buildTree("", GetParam());
 
   auto *S = createEmptyStatement(*Arena);
   EXPECT_TRUE(treeDumpEqual(S, R"txt(
 EmptyStatement Detached synthesized
 `-';' synthesized
   )txt"));
 }
 } // namespace

clang/unittests/Tooling/Syntax/TreeTest.cpp

 namespace {
 
 class TreeTest : public SyntaxTreeTest {
 private:
   Tree *createTree(ArrayRef<const Node *> Children) {
     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);
   }
 
   // Generate Forests by combining `Children` into `ParentCount` Trees.
   //
   // We do this recursively.