[PATCH] D87749: [SyntaxTree][Synthesis] Implement `deepCopy`

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 17 12:47:27 PDT 2020


gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:45
 
+/// Deep copies `N`.
+///
----------------
"Creates a completely independent copy of `N` (a deep copy)."


================
Comment at: clang/lib/Tooling/Syntax/Synthesis.cpp:205
+  if (L->canModify())
+    syntax::FactoryImpl::setCanModify(Leaf);
+
----------------
Since we are creating new leaves, why prohibit their mutation sometimes?

I also don't quite understand the implications of having multiple leaves in a tree that are backed by the same token. I think the algorithm that produces edits can be confused by that.

If you agree, please change the implementation to use `createLeaf` (or call it directly from `deepCopy`).


================
Comment at: clang/unittests/Tooling/Syntax/SynthesisTest.cpp:140
 
+TEST_P(SynthesisTest, Copy_Synthesized) {
+  buildTree("", GetParam());
----------------
Copy => DeepCopy? (also in other tests)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87749



More information about the cfe-commits mailing list