[clang-diff] Improve and test getNodeValue

[johannes]

Summary

Use qualified names if available.

[johannes]

parent changed

[klimek]

lib/Tooling/ASTDiff/ASTDiff.cpp

395 ↗

(On Diff #109309)

Usually we use the initial letter of the type that we now have:
For example, use V (for ValueDecl) here, N for NamedDecl, etc

test/Tooling/clang-diff-ast.cpp

32 ↗

(On Diff #109309)

For my curiosity: why are there semicolons here? Is the format documented somewhere?

[johannes]

NFC renames

[johannes]

renamse, NFC

[johannes]

test/Tooling/clang-diff-ast.cpp

32 ↗

(On Diff #109309)

To avoid collisions (the value should be unique per TypedefDecl in this example), the second one is unnecessary I guess. It is not documented, If it works out I want to move to StmtDataCollector for node comparison and keep these just for visualisation / debugging.

[arphaman]

lib/Tooling/ASTDiff/ASTDiff.cpp

428 ↗

(On Diff #109426)

It looks like you're doing a lot of work for constructors here, but I don't seem them tested in this patch.

[johannes]

add test for delegating initializer and unwritten initializer

[arphaman]

Perfect, thanks!

cfe/trunk/include/clang/Tooling/ASTDiff/ASTDiff.h

struct Node {
   int Depth, Height, Shift = 0;
   ast_type_traits::DynTypedNode ASTNode;
   SmallVector<NodeId, 4> Children;
   ChangeKind Change = None;
 
   ast_type_traits::ASTNodeKind getType() const;
   StringRef getTypeLabel() const;
   bool isLeaf() const { return Children.empty(); }
+  llvm::Optional<StringRef> getIdentifier() const;
+  llvm::Optional<std::string> getQualifiedIdentifier() const;
 };
 
 class ASTDiff {
 public:
   ASTDiff(SyntaxTree &Src, SyntaxTree &Dst, const ComparisonOptions &Options);
   ~ASTDiff();
 
   // Returns the ID of the node that is mapped to the given node in SourceTree.

cfe/trunk/lib/Tooling/ASTDiff/ASTDiff.cpp

public:
   bool isValidNodeId(NodeId Id) const { return Id >= 0 && Id < getSize(); }
   void addNode(Node &N) { Nodes.push_back(N); }
   int getNumberOfDescendants(NodeId Id) const;
   bool isInSubtree(NodeId Id, NodeId SubtreeRoot) const;
   int findPositionInParent(NodeId Id, bool Shifted = false) const;
 
   std::string getNodeValue(NodeId Id) const;
   std::string getNodeValue(const Node &Node) const;
+  std::string getDeclValue(const Decl *D) const;
+  std::string getStmtValue(const Stmt *S) const;
 
 private:
   /// Nodes in preorder.
   std::vector<Node> Nodes;
 
   void initTree();
   void setLeftMostDescendants();
 };
bool TraverseStmt(Stmt *S) {
     RecursiveASTVisitor<PreorderVisitor>::TraverseStmt(S);
     PostTraverse(SavedState);
     return true;
   }
   bool TraverseType(QualType T) { return true; }
 };
 } // end anonymous namespace
 
-SyntaxTree::Impl::Impl(SyntaxTree *Parent, const ASTContext &AST)
-    : Impl(Parent, AST.getTranslationUnitDecl(), AST) {}
-
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, Decl *N, const ASTContext &AST)
     : Parent(Parent), AST(AST) {
   NodeCountVisitor NodeCounter(*this);
   NodeCounter.TraverseDecl(N);
   Nodes.resize(NodeCounter.Count);
   PreorderVisitor PreorderWalker(*this);
   PreorderWalker.TraverseDecl(N);
   initTree();

 }
 
 std::string SyntaxTree::Impl::getNodeValue(NodeId Id) const {
   return getNodeValue(getNode(Id));
 }
 
 std::string SyntaxTree::Impl::getNodeValue(const Node &N) const {
   const DynTypedNode &DTN = N.ASTNode;
-  if (auto *X = DTN.get<BinaryOperator>())
-    return X->getOpcodeStr();
-  if (auto *X = DTN.get<AccessSpecDecl>()) {
-    CharSourceRange Range(X->getSourceRange(), false);
-    return Lexer::getSourceText(Range, AST.getSourceManager(),
-                                AST.getLangOpts());
-  }
-  if (auto *X = DTN.get<IntegerLiteral>()) {
-    SmallString<256> Str;
-    X->getValue().toString(Str, /*Radix=*/10, /*Signed=*/false);
-    return Str.str();
-  }
-  if (auto *X = DTN.get<StringLiteral>())
-    return X->getString();
-  if (auto *X = DTN.get<ValueDecl>())
-    return X->getNameAsString() + "(" + X->getType().getAsString() + ")";
-  if (DTN.get<DeclStmt>() || DTN.get<TranslationUnitDecl>())
-    return "";
+  if (auto *S = DTN.get<Stmt>())
+    return getStmtValue(S);
+  if (auto *D = DTN.get<Decl>())
+    return getDeclValue(D);
+  llvm_unreachable("Fatal: unhandled AST node.\n");
+
+std::string SyntaxTree::Impl::getDeclValue(const Decl *D) const {
   std::string Value;
-  if (auto *X = DTN.get<DeclRefExpr>()) {
-    if (X->hasQualifier()) {
-      llvm::raw_string_ostream OS(Value);
-      PrintingPolicy PP(AST.getLangOpts());
-      X->getQualifier()->print(OS, PP);
+  PrintingPolicy TypePP(AST.getLangOpts());
+  TypePP.AnonymousTagLocations = false;
+
+  if (auto *V = dyn_cast<ValueDecl>(D)) {
+    Value += V->getQualifiedNameAsString() + "(" +
+             V->getType().getAsString(TypePP) + ")";
+    if (auto *C = dyn_cast<CXXConstructorDecl>(D)) {
+      for (auto *Init : C->inits()) {
+        if (!Init->isWritten())
+          continue;
+        if (Init->isBaseInitializer()) {
+          Value += Init->getBaseClass()->getCanonicalTypeInternal().getAsString(
+              TypePP);
+        } else if (Init->isDelegatingInitializer()) {
+          Value += C->getNameAsString();
+        } else {
+          assert(Init->isAnyMemberInitializer());
+          Value += Init->getMember()->getQualifiedNameAsString();
+        }
+        Value += ",";
       }
-    Value += X->getDecl()->getNameAsString();
-    return Value;
     }
-  if (auto *X = DTN.get<NamedDecl>())
-    Value += X->getNameAsString() + ";";
-  if (auto *X = DTN.get<TypedefNameDecl>())
-    return Value + X->getUnderlyingType().getAsString() + ";";
-  if (DTN.get<NamespaceDecl>())
     return Value;
-  if (auto *X = DTN.get<TypeDecl>())
-    if (X->getTypeForDecl())
+  }
+  if (auto *N = dyn_cast<NamedDecl>(D))
+    Value += N->getQualifiedNameAsString() + ";";
+  if (auto *T = dyn_cast<TypedefNameDecl>(D))
+    return Value + T->getUnderlyingType().getAsString(TypePP) + ";";
+  if (auto *T = dyn_cast<TypeDecl>(D))
+    if (T->getTypeForDecl())
       Value +=
-          X->getTypeForDecl()->getCanonicalTypeInternal().getAsString() + ";";
-  if (DTN.get<Decl>())
-    return Value;
-  if (DTN.get<Stmt>())
-    return "";
-  llvm_unreachable("Fatal: unhandled AST node.\n");
+          T->getTypeForDecl()->getCanonicalTypeInternal().getAsString(TypePP) +
+          ";";
+  if (auto *U = dyn_cast<UsingDirectiveDecl>(D))
+    return U->getNominatedNamespace()->getName();
+  if (auto *A = dyn_cast<AccessSpecDecl>(D)) {
+    CharSourceRange Range(A->getSourceRange(), false);
+    return Lexer::getSourceText(Range, AST.getSourceManager(),
+                                AST.getLangOpts());
+  }
+}
+
+std::string SyntaxTree::Impl::getStmtValue(const Stmt *S) const {
+  if (auto *U = dyn_cast<UnaryOperator>(S))
+    return UnaryOperator::getOpcodeStr(U->getOpcode());
+  if (auto *B = dyn_cast<BinaryOperator>(S))
+    return B->getOpcodeStr();
+  if (auto *M = dyn_cast<MemberExpr>(S))
+    return M->getMemberDecl()->getQualifiedNameAsString();
+  if (auto *I = dyn_cast<IntegerLiteral>(S)) {
+    SmallString<256> Str;
+    I->getValue().toString(Str, /*Radix=*/10, /*Signed=*/false);
+    return Str.str();
+  }
+  if (auto *F = dyn_cast<FloatingLiteral>(S)) {
+    SmallString<256> Str;
+    F->getValue().toString(Str);
+    return Str.str();
+  }
+  if (auto *D = dyn_cast<DeclRefExpr>(S))
+    return D->getDecl()->getQualifiedNameAsString();
+  if (auto *String = dyn_cast<StringLiteral>(S))
+    return String->getString();
+  if (auto *B = dyn_cast<CXXBoolLiteralExpr>(S))
+    return B->getValue() ? "true" : "false";
 }
 
 /// Identifies a node in a subtree by its postorder offset, starting at 1.
 struct SNodeId {
   int Id = 0;
 
   explicit SNodeId(int Id) : Id(Id) {}
   explicit SNodeId() = default;

 };
 
 ast_type_traits::ASTNodeKind Node::getType() const {
   return ASTNode.getNodeKind();
 }
 
 StringRef Node::getTypeLabel() const { return getType().asStringRef(); }
 
+llvm::Optional<std::string> Node::getQualifiedIdentifier() const {
+  if (auto *ND = ASTNode.get<NamedDecl>()) {
+    if (ND->getDeclName().isIdentifier())
+      return ND->getQualifiedNameAsString();
+  }
+  return llvm::None;
+}
+
+llvm::Optional<StringRef> Node::getIdentifier() const {
+  if (auto *ND = ASTNode.get<NamedDecl>()) {
+    if (ND->getDeclName().isIdentifier())
+      return ND->getName();
+  }
+  return llvm::None;
+}
+
 namespace {
 // Compares nodes by their depth.
 struct HeightLess {
   const SyntaxTree::Impl &Tree;
   HeightLess(const SyntaxTree::Impl &Tree) : Tree(Tree) {}
   bool operator()(NodeId Id1, NodeId Id2) const {
     return Tree.getNode(Id1).Height < Tree.getNode(Id2).Height;
   }

cfe/trunk/test/Tooling/clang-diff-ast.cpp

 // RUN: clang-diff -ast-dump %s -- -std=c++11 | FileCheck %s
 
 
 // CHECK: {{^}}TranslationUnitDecl(0)
 // CHECK: {{^}} NamespaceDecl: test;(
 namespace test {
 
-// CHECK: {{^}}  FunctionDecl: f(
+// CHECK: {{^}} FunctionDecl: test::f(
 // CHECK: CompoundStmt(
 void f() {
   // CHECK: VarDecl: i(int)(
   // CHECK: IntegerLiteral: 1
   auto i = 1;
+  // CHECK: FloatingLiteral: 1.5(
+  auto r = 1.5;
+  // CHECK: CXXBoolLiteralExpr: true(
+  auto b = true;
   // CHECK: CallExpr(
   // CHECK-NOT: ImplicitCastExpr
-  // CHECK-NEXT: DeclRefExpr: f(
+  // CHECK: DeclRefExpr: test::f(
   f();
+  // CHECK: UnaryOperator: ++(
+  ++i;
   // CHECK: BinaryOperator: =(
   i = i;
 }
 
 } // end namespace test
 
+// CHECK: UsingDirectiveDecl: test(
+using namespace test;
+
 // CHECK: TypedefDecl: nat;unsigned int;(
 typedef unsigned nat;
 // CHECK: TypeAliasDecl: real;double;(
 using real = double;
 
 class Base {
 };
 
-// CHECK: CXXRecordDecl: X;class X;(
+// CHECK: CXXRecordDecl: X;X;(
 class X : Base {
   int m;
-  // CHECK: CXXMethodDecl: foo(const char *(int)
+  // CHECK: CXXMethodDecl: X::foo(const char *(int)
   // CHECK: ParmVarDecl: i(int)(
   const char *foo(int i) {
     if (i == 0)
       // CHECK: StringLiteral: foo(
       return "foo";
     // CHECK-NOT: ImplicitCastExpr
     return 0;
   }
 
   // CHECK: AccessSpecDecl: public(
 public:
-  // CHECK: CXXConstructorDecl: X(void (char, int){{( __attribute__\(\(thiscall\)\))?}})(
-  X(char, int) : Base(), m(0) {
-    // CHECK: MemberExpr(
+  int not_initialized;
+  // All initialized members, bases and delegating initializers are are
+  // appended, separated by commas.
+  // CHECK: CXXConstructorDecl: X::X(void (char, int){{( __attribute__\(\(thiscall\)\))?}})Base,X::m,(
+  X(char c, int) : Base(), m(0) {
+    // CHECK: MemberExpr: X::m(
     int x = m;
   }
+  // CHECK: CXXConstructorDecl: X::X(void (char))X,(
+  X(char s) : X(s, 4) {}
 };
 
 #define M (void)1
 #define MA(a, b) (void)a, b
 // CHECK: FunctionDecl
 // CHECK-NEXT: CompoundStmt
 void macros() {
   M;

cfe/trunk/test/Tooling/clang-diff-basic.cpp

 // RUN: clang-diff -dump-matches %S/Inputs/clang-diff-basic-src.cpp %s -- | FileCheck %s
 
 // CHECK: Match TranslationUnitDecl{{.*}} to TranslationUnitDecl
 // CHECK: Match NamespaceDecl: src{{.*}} to NamespaceDecl: dst
 namespace dst {
 // CHECK-NOT: Match NamespaceDecl: src{{.*}} to NamespaceDecl: inner
 namespace inner {
 void foo() {
   // CHECK: Match IntegerLiteral: 321{{.*}} to IntegerLiteral: 322
   int x = 322;
 }
 }
 
-// CHECK: Match DeclRefExpr: foo{{.*}} to DeclRefExpr: inner::foo
+// CHECK: Match DeclRefExpr: src::foo{{.*}} to DeclRefExpr: dst::inner::foo
 void main() { inner::foo(); }
 
 // CHECK: Match StringLiteral: foo{{.*}} to StringLiteral: foo
 const char *b = "f" "o" "o";
 
 // unsigned is canonicalized to unsigned int
-// CHECK: Match TypedefDecl: nat;unsigned int;{{.*}} to TypedefDecl: nat;unsigned int;
+// CHECK: Match TypedefDecl: src::nat;unsigned int;{{.*}} to TypedefDecl: dst::nat;unsigned int;
 typedef unsigned nat;
 
-// CHECK: Match VarDecl: p(int){{.*}} to VarDecl: prod(double)
-// CHECK: Update VarDecl: p(int){{.*}} to prod(double)
+// CHECK: Match VarDecl: src::p(int){{.*}} to VarDecl: dst::prod(double)
+// CHECK: Update VarDecl: src::p(int){{.*}} to dst::prod(double)
 // CHECK: Match BinaryOperator: *{{.*}} to BinaryOperator: *
 double prod = 1 * 2 * 10;
 // CHECK: Update DeclRefExpr
 int squared = prod * prod;
 
 class X {
   const char *foo(int i) {
     if (i == 0)

 
 // CHECK: Move CompoundStmt{{.*}} into CompoundStmt
 void m() { { int x = 0 + 0 + 0; } }
 // CHECK: Update and Move IntegerLiteral: 7{{.*}} into BinaryOperator: +({{.*}}) at 1
 int um = 1 + 7;
 
 namespace {
 // match with parents of different type
-// CHECK: Match FunctionDecl: f1{{.*}} to FunctionDecl: f1
+// CHECK: Match FunctionDecl: f1{{.*}} to FunctionDecl: (anonymous namespace)::f1
 void f1() {{ (void) __func__;;; }}
 }
 
 // CHECK: Delete AccessSpecDecl: public
 // CHECK: Delete CXXMethodDecl

cfe/trunk/test/Tooling/clang-diff-bottomup.cpp

 
 #else
 
 // Jaccard similarity threshold is 0.5.
 
 void f1() {
 // CompoundStmt: 3 matched descendants, subtree sizes 4 and 5
 // Jaccard similarity = 3 / (4 + 5 - 3) = 3 / 6 >= 0.5
-// CHECK: Match FunctionDecl: f1(void (void))(1) to FunctionDecl: f1(void (void))(1)
+// CHECK: Match FunctionDecl: f1(void ())(1) to FunctionDecl: f1(void ())(1)
 // CHECK: Match CompoundStmt(2) to CompoundStmt(2)
 // CHECK: Match CompoundStmt(4) to CompoundStmt(3)
 // CHECK: Match CompoundStmt(5) to CompoundStmt(4)
 // CHECK: Match NullStmt(6) to NullStmt(5)
   {{;}} ;;
 }
 
 void f2() {

cfe/trunk/test/Tooling/clang-diff-html.test

 // RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -- | FileCheck %s
 
 // CHECK: <pre><div id='L' class='code'><span id='L0' tid='R0' title='TranslationUnitDecl
 // CHECK-NEXT: 0 -> 0'>
 
 // match, update
 // CHECK: <span id='L[[L:[0-9]+]]' tid='R[[R:[0-9]+]]' title='NamespaceDecl
 // CHECK-NEXT: [[L]] -> [[R]]
 // CHECK-NEXT: src;' class='u'>namespace src {
 
 // match, move
 // CHECK: <span id='L[[L:[0-9]+]]' tid='R[[R:[0-9]+]]' title='FunctionDecl
 // CHECK-NEXT: [[L]] -> [[R]]
-// CHECK-NEXT: foo(void (void))' class='m'>void foo()
+// CHECK-NEXT: src::foo(void ())' class='u m'>void foo()
 
 // match
 // CHECK: <span id='L[[L:[0-9]+]]' tid='R[[R:[0-9]+]]' title='FunctionDecl
 // CHECK-NEXT: [[L]] -> [[R]]
-// CHECK-NEXT: main(void (void))'>void main()
+// CHECK-NEXT: src::main(void ())' class='u'>void main()
 
 // deletion
 // CHECK: <span id='L[[L:[0-9]+]]' tid='R-1' title='IntegerLiteral
 // CHECK-NEXT: [[L]] -> -1
 // CHECK-NEXT: 4' class='d'>4</span>
 
 // update + move
 // CHECK: 2' class='u m'>2</span>

cfe/trunk/test/Tooling/clang-diff-opt.cpp

 
 void f3() {
   // Jaccard similarity = 8 / (11 + 11 - 8) >= 0.5
   // As the subtrees are bigger than 10 nodes, the optimal algorithm will not
   // be run.
   // CHECK: Delete NullStmt(22)
   ;; {{;;;;;;}}
 }
+ 
 #endif

cfe/trunk/test/Tooling/clang-diff-topdown.cpp

 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- | FileCheck %s
+// RUN: clang-diff -dump-matches -stop-after=topdown %t.src.cpp %t.dst.cpp -- -std=c++11 | FileCheck %s
 //
 // Test the top-down matching of identical subtrees only.
 
 #ifndef DEST
 
 void f1()
 {
   // Match some subtree of height greater than 2.

cfe/trunk/tools/clang-diff/ClangDiff.cpp

static void printNodeAttributes(raw_ostream &OS, diff::SyntaxTree &Tree,
   }
 }
 
 static void printNodeAsJson(raw_ostream &OS, diff::SyntaxTree &Tree,
                             diff::NodeId Id) {
   const diff::Node &N = Tree.getNode(Id);
   OS << "{";
   printNodeAttributes(OS, Tree, Id);
+  auto Identifier = N.getIdentifier();
+  auto QualifiedIdentifier = N.getQualifiedIdentifier();
+  if (Identifier) {
+    OS << R"(,"identifier":")";
+    printJsonString(OS, *Identifier);
+    OS << R"(")";
+    if (QualifiedIdentifier && *Identifier != *QualifiedIdentifier) {
+      OS << R"(,"qualified_identifier":")";
+      printJsonString(OS, *QualifiedIdentifier);
+      OS << R"(")";
+    }
+  }
   OS << R"(,"children":[)";
   if (N.Children.size() > 0) {
     printNodeAsJson(OS, Tree, N.Children[0]);
     for (size_t I = 1, E = N.Children.size(); I < E; ++I) {
       OS << ",";
       printNodeAsJson(OS, Tree, N.Children[I]);
     }
   }