[PATCH] D89147: [SyntaxTree] Improve the signature of `replaceChildRangeLowLevel`.

2020-10-12 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 297511.
eduucaldas added a comment.

Add asserts to `MutationsImpl::remove`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89147

Files:
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Mutations.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -92,50 +92,84 @@
   this->FirstChild = Child;
 }
 
-void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
- Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+std::vector>
+syntax::Tree::replaceChildRangeLowLevel(
+Node *BeforeBegin, Node *End,
+ArrayRef> NewRange) {
+  assert(!BeforeBegin || BeforeBegin->Parent == this &&
+ "`BeforeBegin` is not a child of `this`");
+  assert(!End || End->Parent == this && "`End` is not a child of `this`");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (const auto  : NewRange) {
+auto *N = NodeAndRole.first;
 assert(N->Parent == nullptr);
-assert(N->getRole() != NodeRole::Detached && "Roles must be set");
-// FIXME: sanity-check the role.
+assert(N->getRole() == NodeRole::Detached && "Roles must not be set");
+
+auto Role = NodeAndRole.second;
+assert(Role != NodeRole::Detached);
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`");
 #endif
 
-  // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  auto GetBegin = [,  = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
+
+  // Extract old nodes.
+  std::vector> ExtractedChildren;
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
+ExtractedChildren.push_back({N, N->getRole()});
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
-  // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
+  // If any modification occurred mark them.
+  if (!ExtractedChildren.empty() || !NewRange.empty())
+for (auto *T = this; T && T->Original; T = T->Parent)
+  T->Original = false;
 
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  // Attach new children to the replaced range.
+
+  if (NewRange.empty()) {
+GetBegin() = End;
+return ExtractedChildren;
+  }
+  GetBegin() = NewRange.front().first;
+  NewRange.back().first->NextSibling = End;
+
+  for (const auto  : NewRange) {
+NodeAndRole.first->setRole(NodeAndRole.second);
+NodeAndRole.first->Parent = this;
+  }
+
+  for (const auto *It = NewRange.begin(); It != std::prev(NewRange.end());
+   ++It) {
+auto *Node = It->first;
+auto *Next = std::next(It)->first;
+Node->NextSibling = Next;
   }
 
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  return ExtractedChildren;
 }
 
 namespace {
Index: clang/lib/Tooling/Syntax/Mutations.cpp
===
--- clang/lib/Tooling/Syntax/Mutations.cpp
+++ clang/lib/Tooling/Syntax/Mutations.cpp
@@ -23,6 +23,17 @@
 
 using namespace clang;
 
+static syntax::Node *findPrevious(syntax::Node *N) {
+  if (N->getParent()->getFirstChild() == N)
+return nullptr;
+  for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
+   C = C->getNextSibling()) {
+if (C->getNextSibling() == N)
+  return C;
+  }
+  llvm_unreachable("could not find a child node");
+}
+
 // This class has access to the internals of tree nodes. Its sole purpose is to
 // define helpers that allow implementing the high-level mutation operations.
 class syntax::MutationsImpl {
@@ -30,14 +41,15 @@
   /// Add a new node with a specified role.
   static void addAfter(syntax::Node *Anchor, syntax::Node *New, NodeRole Role) {
 assert(Anchor != nullptr);
+assert(Anchor->Parent != nullptr);
 assert(New->Parent == nullptr);
 

[PATCH] D89147: [SyntaxTree] Improve the signature of `replaceChildRangeLowLevel`.

2020-10-10 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 297396.
eduucaldas added a comment.

Fix whitespacing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89147

Files:
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Mutations.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -92,50 +92,84 @@
   this->FirstChild = Child;
 }
 
-void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
- Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+std::vector>
+syntax::Tree::replaceChildRangeLowLevel(
+Node *BeforeBegin, Node *End,
+ArrayRef> NewRange) {
+  assert(!BeforeBegin || BeforeBegin->Parent == this &&
+ "`BeforeBegin` is not a child of `this`");
+  assert(!End || End->Parent == this && "`End` is not a child of `this`");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (const auto  : NewRange) {
+auto *N = NodeAndRole.first;
 assert(N->Parent == nullptr);
-assert(N->getRole() != NodeRole::Detached && "Roles must be set");
-// FIXME: sanity-check the role.
+assert(N->getRole() == NodeRole::Detached && "Roles must not be set");
+
+auto Role = NodeAndRole.second;
+assert(Role != NodeRole::Detached);
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`");
 #endif
 
-  // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  auto GetBegin = [,  = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
+
+  // Extract old nodes.
+  std::vector> ExtractedChildren;
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
+ExtractedChildren.push_back({N, N->getRole()});
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
-  // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
+  // If any modification occurred mark them.
+  if (!ExtractedChildren.empty() || !NewRange.empty())
+for (auto *T = this; T && T->Original; T = T->Parent)
+  T->Original = false;
 
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  // Attach new children to the replaced range.
+
+  if (NewRange.empty()) {
+GetBegin() = End;
+return ExtractedChildren;
+  }
+  GetBegin() = NewRange.front().first;
+  NewRange.back().first->NextSibling = End;
+
+  for (const auto  : NewRange) {
+NodeAndRole.first->setRole(NodeAndRole.second);
+NodeAndRole.first->Parent = this;
+  }
+
+  for (const auto *It = NewRange.begin(); It != std::prev(NewRange.end());
+   ++It) {
+auto *Node = It->first;
+auto *Next = std::next(It)->first;
+Node->NextSibling = Next;
   }
 
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  return ExtractedChildren;
 }
 
 namespace {
Index: clang/lib/Tooling/Syntax/Mutations.cpp
===
--- clang/lib/Tooling/Syntax/Mutations.cpp
+++ clang/lib/Tooling/Syntax/Mutations.cpp
@@ -23,6 +23,17 @@
 
 using namespace clang;
 
+static syntax::Node *findPrevious(syntax::Node *N) {
+  if (N->getParent()->getFirstChild() == N)
+return nullptr;
+  for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
+   C = C->getNextSibling()) {
+if (C->getNextSibling() == N)
+  return C;
+  }
+  llvm_unreachable("could not find a child node");
+}
+
 // This class has access to the internals of tree nodes. Its sole purpose is to
 // define helpers that allow implementing the high-level mutation operations.
 class syntax::MutationsImpl {
@@ -30,14 +41,15 @@
   /// Add a new node with a specified role.
   static void addAfter(syntax::Node *Anchor, syntax::Node *New, NodeRole Role) {
 assert(Anchor != nullptr);
+assert(Anchor->Parent != nullptr);
 assert(New->Parent == nullptr);
 assert(New->NextSibling == 

[PATCH] D89147: [SyntaxTree] Improve the signature of `replaceChildRangeLowLevel`.

2020-10-10 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 297395.
eduucaldas added a comment.

Add role sanity-check. Introduce `GetBegin()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89147

Files:
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Mutations.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -92,50 +92,84 @@
   this->FirstChild = Child;
 }
 
-void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
- Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+std::vector>
+syntax::Tree::replaceChildRangeLowLevel(
+Node *BeforeBegin, Node *End,
+ArrayRef> NewRange) {
+  assert(!BeforeBegin || BeforeBegin->Parent == this &&
+ "`BeforeBegin` is not a child of `this`");
+  assert(!End || End->Parent == this && "`End` is not a child of `this`");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (const auto  : NewRange) {
+auto *N = NodeAndRole.first;
 assert(N->Parent == nullptr);
-assert(N->getRole() != NodeRole::Detached && "Roles must be set");
-// FIXME: sanity-check the role.
+assert(N->getRole() == NodeRole::Detached && "Roles must not be set");
+
+auto Role = NodeAndRole.second;
+assert(Role != NodeRole::Detached);
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`");
+
 #endif
 
-  // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  auto GetBegin = [,  = this->FirstChild]() -> Node *& {
+return BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  };
+
+  // Extract old nodes.
+  std::vector> ExtractedChildren;
+  for (auto *N = GetBegin(); N != End;) {
 auto *Next = N->NextSibling;
+ExtractedChildren.push_back({N, N->getRole()});
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
-  // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
+  // If any modification occurred mark this and its ancestors as modified.
+  if (!ExtractedChildren.empty() || !NewRange.empty())
+for (auto *T = this; T && T->Original; T = T->Parent)
+  T->Original = false;
 
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  // Attach new children to the replaced range.
+  if (NewRange.empty()) {
+GetBegin() = End;
+return ExtractedChildren;
+  }
+  GetBegin() = NewRange.front().first;
+  NewRange.back().first->NextSibling = End;
+
+  for (const auto  : NewRange) {
+NodeAndRole.first->setRole(NodeAndRole.second);
+NodeAndRole.first->Parent = this;
+  }
+
+  for (const auto *It = NewRange.begin(); It != std::prev(NewRange.end());
+   ++It) {
+auto *Node = It->first;
+auto *Next = std::next(It)->first;
+Node->NextSibling = Next;
   }
 
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  return ExtractedChildren;
 }
 
 namespace {
Index: clang/lib/Tooling/Syntax/Mutations.cpp
===
--- clang/lib/Tooling/Syntax/Mutations.cpp
+++ clang/lib/Tooling/Syntax/Mutations.cpp
@@ -23,6 +23,17 @@
 
 using namespace clang;
 
+static syntax::Node *findPrevious(syntax::Node *N) {
+  if (N->getParent()->getFirstChild() == N)
+return nullptr;
+  for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
+   C = C->getNextSibling()) {
+if (C->getNextSibling() == N)
+  return C;
+  }
+  llvm_unreachable("could not find a child node");
+}
+
 // This class has access to the internals of tree nodes. Its sole purpose is to
 // define helpers that allow implementing the high-level mutation operations.
 class syntax::MutationsImpl {
@@ -30,14 +41,15 @@
   /// Add a new node with a specified role.
   static void addAfter(syntax::Node *Anchor, syntax::Node *New, NodeRole Role) {
 assert(Anchor != nullptr);
+assert(Anchor->Parent != nullptr);
 

[PATCH] D89147: [SyntaxTree] Improve the signature of `replaceChildRangeLowLevel`.

2020-10-10 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 297389.
eduucaldas added a comment.

Add reachability assertions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89147

Files:
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Mutations.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -92,50 +92,87 @@
   this->FirstChild = Child;
 }
 
-void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
- Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+std::vector>
+syntax::Tree::replaceChildRangeLowLevel(
+Node *BeforeBegin, Node *End,
+ArrayRef> NewRange) {
+  assert(!BeforeBegin || BeforeBegin->Parent == this &&
+ "`BeforeBegin` is not a child of `this`");
+  assert(!End || End->Parent == this && "`End` is not a child of `this`");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (const auto  : NewRange) {
+auto *N = NodeAndRole.first;
 assert(N->Parent == nullptr);
-assert(N->getRole() != NodeRole::Detached && "Roles must be set");
-// FIXME: sanity-check the role.
+assert(N->getRole() == NodeRole::Detached && "Roles must not be set");
   }
+
+  auto Reachable = [](Node *From, Node *N) {
+if (!N)
+  return true;
+for (auto *It = From; It; It = It->NextSibling)
+  if (It == N)
+return true;
+return false;
+  };
+  assert(Reachable(FirstChild, BeforeBegin) &&
+ "`BeforeBegin` is not reachable");
+  assert(Reachable(BeforeBegin ? BeforeBegin->NextSibling : FirstChild, End) &&
+ "`End` is not after `BeforeBegin`");
+
 #endif
 
-  // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  // Extract children to be replaced.
+  std::vector> ExtractedChildren;
+  auto *Begin = BeforeBegin ? BeforeBegin->NextSibling : FirstChild;
+  for (auto *N = Begin; N != End;) {
 auto *Next = N->NextSibling;
+ExtractedChildren.push_back({N, N->getRole()});
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
+
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
-  // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
+  // If any modification occurred mark this and its ancestors as modified.
+  if (!ExtractedChildren.empty() || !NewRange.empty())
+for (auto *T = this; T && T->Original; T = T->Parent)
+  T->Original = false;
 
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  if (NewRange.empty()) {
+BeforeBegin->NextSibling = End;
+return ExtractedChildren;
   }
 
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  // `NewRange` becomes children of this Tree.
+  for (const auto  : NewRange) {
+NodeAndRole.first->setRole(NodeAndRole.second);
+NodeAndRole.first->Parent = this;
+  }
+
+  // `NewRange` nodes are siblings.
+  for (const auto *It = NewRange.begin(); It != std::prev(NewRange.end());
+   ++It) {
+auto *Node = It->first;
+auto *Next = std::next(It)->first;
+Node->NextSibling = Next;
+  }
+  // Attach new children to the end of the replaced range.
+  NewRange.back().first->NextSibling = End;
+
+  // New children are now the beginning of the replaced range.
+  auto *NewBegin = NewRange.front().first;
+  if (BeforeBegin)
+BeforeBegin->NextSibling = NewBegin;
+  else
+FirstChild = NewBegin;
+
+  return ExtractedChildren;
 }
 
 namespace {
Index: clang/lib/Tooling/Syntax/Mutations.cpp
===
--- clang/lib/Tooling/Syntax/Mutations.cpp
+++ clang/lib/Tooling/Syntax/Mutations.cpp
@@ -23,6 +23,17 @@
 
 using namespace clang;
 
+static syntax::Node *findPrevious(syntax::Node *N) {
+  if (N->getParent()->getFirstChild() == N)
+return nullptr;
+  for (syntax::Node *C = N->getParent()->getFirstChild(); C != nullptr;
+   C = C->getNextSibling()) {
+if (C->getNextSibling() == N)
+  return C;
+  }
+  llvm_unreachable("could not find a child node");
+}
+
 // This class has access to the internals of tree nodes. Its sole purpose is to
 // define helpers that allow implementing the high-level mutation operations.
 class syntax::MutationsImpl {
@@ -30,14 +41,15 @@
   /// Add a new node with a specified role.
   static void addAfter(syntax::Node 

[PATCH] D89147: [SyntaxTree] Improve the signature of `replaceChildRangeLowLevel`.

2020-10-09 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.

Previously `replaceChildRangeLowLevel` took the new child range as a
`Node* New`.  `New` was expected to have siblings attached already, and
thus it was interpreted as a range. Additionally, the role of `New` and
its siblings were expected to be set prior to calling the function.  As
a result the `New` argument broke the invariant `New->Parent == nullptr`
<=> `New->Role == Detached`, at call site.  We change the signature of
`replaceChildRangeLowLevel` to take instead an
`ArrayRef>`, and thus move the burden of
setting siblings and roles from the user to the member function.

Moreover, `replaceChildRangeLowLevel` returns now a vector of the
replaced range, following the "law of useful returns". Previously, in
order to reuse the replaced elements the user needed to get pointers to
those elements and before calling the function.

We also fixed some minor bugs in `addAfter`, and added more asserts to
the new `replaceChildRangeLowLevel`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89147

Files:
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/Mutations.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -92,50 +92,76 @@
   this->FirstChild = Child;
 }
 
-void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
- Node *New) {
-  assert(!BeforeBegin || BeforeBegin->Parent == this);
+std::vector>
+syntax::Tree::replaceChildRangeLowLevel(
+Node *BeforeBegin, Node *End,
+ArrayRef> NewRange) {
+  assert(!BeforeBegin || BeforeBegin->Parent == this &&
+ "`BeforeBegin` is not a child of `this`");
+  assert(!End || End->Parent == this && "`End` is not a child of `this`");
 
 #ifndef NDEBUG
-  for (auto *N = New; N; N = N->getNextSibling()) {
+  for (const auto  : NewRange) {
+auto *N = NodeAndRole.first;
 assert(N->Parent == nullptr);
-assert(N->getRole() != NodeRole::Detached && "Roles must be set");
-// FIXME: sanity-check the role.
+assert(N->getRole() == NodeRole::Detached && "Roles must not be set");
   }
+  // TODO: assert that both `BeforeBegin` and `End` are children of parent, and
+  // that `End` is "after" `BeforeBegin`.
+
 #endif
 
-  // Detach old nodes.
-  for (auto *N = !BeforeBegin ? FirstChild : BeforeBegin->getNextSibling();
-   N != End;) {
+  // Extract children to be replaced.
+  std::vector> ExtractedChildren;
+  auto *Begin = BeforeBegin ? BeforeBegin->getNextSibling() : FirstChild;
+  for (auto *N = Begin; N != End;) {
 auto *Next = N->NextSibling;
+ExtractedChildren.push_back({N, N->getRole()});
 
 N->setRole(NodeRole::Detached);
 N->Parent = nullptr;
 N->NextSibling = nullptr;
+
 if (N->Original)
-  traverse(N, [&](Node *C) { C->Original = false; });
+  traverse(N, [](Node *C) { C->Original = false; });
 
 N = Next;
   }
 
-  // Attach new nodes.
-  if (BeforeBegin)
-BeforeBegin->NextSibling = New ? New : End;
-  else
-FirstChild = New ? New : End;
+  // If any modification occurred mark this and its ancestors as modified.
+  if (!ExtractedChildren.empty() || !NewRange.empty())
+for (auto *T = this; T && T->Original; T = T->Parent)
+  T->Original = false;
 
-  if (New) {
-auto *Last = New;
-for (auto *N = New; N != nullptr; N = N->getNextSibling()) {
-  Last = N;
-  N->Parent = this;
-}
-Last->NextSibling = End;
+  if (NewRange.empty()) {
+BeforeBegin->NextSibling = End;
+return ExtractedChildren;
+  }
+
+  // `NewRange` becomes children of this Tree.
+  for (const auto  : NewRange) {
+NodeAndRole.first->setRole(NodeAndRole.second);
+NodeAndRole.first->Parent = this;
+  }
+
+  // `NewRange` nodes are siblings.
+  for (const auto *It = NewRange.begin(); It != std::prev(NewRange.end());
+   ++It) {
+auto *Node = It->first;
+auto *Next = std::next(It)->first;
+Node->NextSibling = Next;
   }
+  // Attach new children to the end of the replaced range.
+  NewRange.back().first->NextSibling = End;
+
+  // New children are now the beginning of the replaced range.
+  auto *NewBegin = NewRange.front().first;
+  if (BeforeBegin)
+BeforeBegin->NextSibling = NewBegin;
+  else
+FirstChild = NewBegin;
 
-  // Mark the node as modified.
-  for (auto *T = this; T && T->Original; T = T->Parent)
-T->Original = false;
+  return ExtractedChildren;
 }
 
 namespace {
Index: clang/lib/Tooling/Syntax/Mutations.cpp
===
--- clang/lib/Tooling/Syntax/Mutations.cpp
+++ clang/lib/Tooling/Syntax/Mutations.cpp
@@ -23,6 +23,17 @@
 
 using namespace clang;