johannes created this revision.
Herald added a subscriber: klimek.

This makes findPositionInParent() a member of Node.  Additionally, the
function that computes the new position that is dependent on
insertions and deletions of siblings, is now a member of
ASTDiff::Impl.


https://reviews.llvm.org/D39649

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===================================================================
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -434,7 +434,7 @@
       OS << "None";
     else
       printNode(OS, DstTree, *Dst.getParent());
-    OS << " at " << DstTree.findPositionInParent(Dst) << "\n";
+    OS << " at " << Dst.findPositionInParent() << "\n";
     break;
   }
 }
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===================================================================
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -83,6 +83,8 @@
   // Tries to match any yet unmapped nodes, in a bottom-up fashion.
   void matchBottomUp();
 
+  int findNewPosition(NodeRef N) const;
+
   const ComparisonOptions &Options;
 
   friend class ZhangShashaMatcher;
@@ -141,7 +143,6 @@
   Node &getMutableNode(NodeRef N) { return getMutableNode(N.getId()); }
   int getNumberOfDescendants(NodeRef N) const;
   bool isInSubtree(NodeRef N, NodeRef SubtreeRoot) const;
-  int findPositionInParent(NodeRef Id, bool Shifted = false) const;
 
   std::string getRelativeName(const NamedDecl *ND,
                               const DeclContext *Context) const;
@@ -339,23 +340,6 @@
          N.getId() <= SubtreeRoot.RightMostDescendant;
 }
 
-int SyntaxTree::Impl::findPositionInParent(NodeRef N, bool Shifted) const {
-  if (!N.getParent())
-    return 0;
-  NodeRef Parent = *N.getParent();
-  const auto &Siblings = Parent.Children;
-  int Position = 0;
-  for (size_t I = 0, E = Siblings.size(); I < E; ++I) {
-    if (Shifted)
-      Position += getNode(Siblings[I]).Shift;
-    if (Siblings[I] == N.getId()) {
-      Position += I;
-      return Position;
-    }
-  }
-  llvm_unreachable("Node not found in parent's children.");
-}
-
 // Returns the qualified name of ND. If it is subordinate to Context,
 // then the prefix of the latter is removed from the returned value.
 std::string
@@ -724,6 +708,14 @@
   return {&Tree, isLeaf() ? nullptr : &Children[0] + Children.size()};
 }
 
+int Node::findPositionInParent() const {
+  if (!getParent())
+    return 0;
+  const ArrayRef<NodeId> &Siblings = getParent()->Children;
+  return std::find(Siblings.begin(), Siblings.end(), getId()) -
+         Siblings.begin();
+}
+
 namespace {
 // Compares nodes by their depth.
 struct HeightLess {
@@ -950,8 +942,8 @@
     if (!getDst(N1))
       continue;
     NodeRef N2 = *getDst(N1);
-    if (!haveSameParents(N1, N2) || T1.findPositionInParent(N1, true) !=
-                                        T2.findPositionInParent(N2, true)) {
+    if (!haveSameParents(N1, N2) ||
+        findNewPosition(N1) != findNewPosition(N2)) {
       T1.getMutableNode(N1).Shift -= 1;
       T2.getMutableNode(N2).Shift -= 1;
     }
@@ -962,8 +954,8 @@
     NodeRef N1 = *getSrc(N2);
     Node &MutableN1 = T1.getMutableNode(N1);
     Node &MutableN2 = T2.getMutableNode(N2);
-    if (!haveSameParents(N1, N2) || T1.findPositionInParent(N1, true) !=
-                                        T2.findPositionInParent(N2, true)) {
+    if (!haveSameParents(N1, N2) ||
+        findNewPosition(N1) != findNewPosition(N2)) {
       MutableN1.Change = MutableN2.Change = Move;
     }
     if (T1.getNodeValue(N1) != T2.getNodeValue(N2)) {
@@ -995,6 +987,18 @@
                                         : nullptr;
 }
 
+int ASTDiff::Impl::findNewPosition(NodeRef N) const {
+  if (!N.getParent())
+    return 0;
+  int Position = N.findPositionInParent();
+  for (NodeRef Sibling : *N.getParent()) {
+    Position += Sibling.Shift;
+    if (&Sibling == &N)
+      return Position;
+  }
+  llvm_unreachable("Node not found amongst parent's children.");
+}
+
 ASTDiff::ASTDiff(SyntaxTree &T1, SyntaxTree &T2,
                  const ComparisonOptions &Options)
     : DiffImpl(llvm::make_unique<Impl>(*T1.TreeImpl, *T2.TreeImpl, Options)) {}
@@ -1022,10 +1026,6 @@
 }
 SyntaxTree::PreorderIterator SyntaxTree::end() const { return TreeImpl->end(); }
 
-int SyntaxTree::findPositionInParent(NodeRef N) const {
-  return TreeImpl->findPositionInParent(N);
-}
-
 std::pair<unsigned, unsigned>
 SyntaxTree::getSourceRangeOffsets(NodeRef N) const {
   const SourceManager &SM = TreeImpl->AST.getSourceManager();
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===================================================================
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -90,7 +90,6 @@
   PreorderIterator end() const;
 
   NodeRef getNode(NodeId Id) const;
-  int findPositionInParent(NodeRef Node) const;
 
   // Returns the starting and ending offset of the node in its source file.
   std::pair<unsigned, unsigned> getSourceRangeOffsets(NodeRef N) const;
@@ -128,6 +127,8 @@
 
   NodeRefIterator begin() const;
   NodeRefIterator end() const;
+
+  int findPositionInParent() const;
 };
 
 struct NodeRefIterator {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to