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

Nodes that are expanded from some macro are given a special
"Macro" kind instead of an ASTNodeKind. They are compared by
their textual value including arguments, before expansion.


https://reviews.llvm.org/D36997

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/Inputs/clang-diff-basic-src.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===================================================================
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -483,7 +483,7 @@
     std::unique_ptr<ASTUnit> AST = getAST(CommonCompilations, SourcePath);
     if (!AST)
       return 1;
-    diff::SyntaxTree Tree(AST->getASTContext());
+    diff::SyntaxTree Tree(*AST);
     if (ASTDump) {
       printTree(llvm::outs(), Tree);
       return 0;
@@ -561,8 +561,8 @@
       return 1;
     }
   }
-  diff::SyntaxTree SrcTree(Src->getASTContext());
-  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::SyntaxTree SrcTree(*Src);
+  diff::SyntaxTree DstTree(*Dst);
   diff::ASTDiff Diff(SrcTree, DstTree, Options);
 
   if (HtmlDiff) {
Index: test/Tooling/clang-diff-basic.cpp
===================================================================
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -53,5 +53,20 @@
 void f1() {{ (void) __func__;;; }}
 }
 
+// macros are always compared by their full textual value
+
+#define M1 return 1 + 1
+#define M2 return 2 * 2
+#define F(a, b) return a + b;
+
+int f2() {
+  // CHECK: Match Macro: M1{{.*}} to Macro: M1
+  M1;
+  // CHECK: Update Macro: M1{{.*}} to M2
+  M2;
+  // CHECK: Update Macro: F(1, 1){{.*}} to F(1, /*b=*/1)
+  F(1, /*b=*/1);
+}
+
 // CHECK: Delete AccessSpecDecl: public
 // CHECK: Delete CXXMethodDecl
Index: test/Tooling/clang-diff-ast.cpp
===================================================================
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -66,12 +66,14 @@
 };
 
 #define M (void)1
-#define MA(a, b) (void)a, b
-// CHECK: FunctionDecl
-// CHECK-NEXT: CompoundStmt
+#define F(a, b) (void)a, b
 void macros() {
+  // CHECK: Macro: M(
   M;
-  MA(1, 2);
+  // two expressions, therefore it occurs twice
+  // CHECK-NEXT: Macro: F(1, 2)(
+  // CHECK-NEXT: Macro: F(1, 2)(
+  F(1, 2);
 }
 
 #ifndef GUARD
Index: test/Tooling/Inputs/clang-diff-basic-src.cpp
===================================================================
--- test/Tooling/Inputs/clang-diff-basic-src.cpp
+++ test/Tooling/Inputs/clang-diff-basic-src.cpp
@@ -31,3 +31,13 @@
 int um = 1 * 2 + 3;
 
 void f1() {{ (void) __func__;;; }}
+
+#define M1 return 1 + 1
+#define M2 return 1 + 2
+#define F(a, b) return a + b;
+
+int f2() {
+  M1;
+  M1;
+  F(1, 1);
+}
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===================================================================
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -120,21 +120,21 @@
 class SyntaxTree::Impl {
 public:
   /// Constructs a tree from an AST node.
-  Impl(SyntaxTree *Parent, Decl *N, ASTContext &AST);
-  Impl(SyntaxTree *Parent, Stmt *N, ASTContext &AST);
+  Impl(SyntaxTree *Parent, Decl *N, ASTUnit &AST);
+  Impl(SyntaxTree *Parent, Stmt *N, ASTUnit &AST);
   template <class T>
   Impl(SyntaxTree *Parent,
        typename std::enable_if<std::is_base_of<Stmt, T>::value, T>::type *Node,
-       ASTContext &AST)
+       ASTUnit &AST)
       : Impl(Parent, dyn_cast<Stmt>(Node), AST) {}
   template <class T>
   Impl(SyntaxTree *Parent,
        typename std::enable_if<std::is_base_of<Decl, T>::value, T>::type *Node,
-       ASTContext &AST)
+       ASTUnit &AST)
       : Impl(Parent, dyn_cast<Decl>(Node), AST) {}
 
   SyntaxTree *Parent;
-  ASTContext &AST;
+  ASTUnit &AST;
   std::vector<NodeId> Leaves;
   // Maps preorder indices to postorder ones.
   std::vector<int> PostorderIds;
@@ -173,39 +173,57 @@
 static bool isSpecializedNodeExcluded(const Decl *D) { return D->isImplicit(); }
 static bool isSpecializedNodeExcluded(const Stmt *S) { return false; }
 
-template <class T>
-static bool isNodeExcluded(const SourceManager &SrcMgr, T *N) {
+template <class T> static bool isNodeExcluded(ASTUnit &AST, T *N) {
+  const SourceManager &SrcMgr = AST.getSourceManager();
   if (!N)
     return true;
   SourceLocation SLoc = N->getLocStart();
   if (SLoc.isValid()) {
     // Ignore everything from other files.
     if (!SrcMgr.isInMainFile(SLoc))
       return true;
-    // Ignore macros.
-    if (SLoc != SrcMgr.getSpellingLoc(SLoc))
+    const Preprocessor &PP = AST.getPreprocessor();
+    if (SLoc.isMacroID() && !PP.isAtStartOfMacroExpansion(SLoc))
       return true;
   }
   return isSpecializedNodeExcluded(N);
 }
 
+static SourceRange getSourceRange(const ASTUnit &AST, const DynTypedNode &DTN) {
+  const SourceManager &SrcMgr = AST.getSourceManager();
+  SourceRange Range = DTN.getSourceRange();
+  SourceLocation BeginLoc = Range.getBegin();
+  SourceLocation EndLoc;
+  if (BeginLoc.isMacroID())
+    EndLoc = SrcMgr.getExpansionRange(BeginLoc).second;
+  else
+    EndLoc = Range.getEnd();
+  EndLoc = Lexer::getLocForEndOfToken(EndLoc, /*Offset=*/0, SrcMgr,
+                                      AST.getLangOpts());
+  if (auto *ThisExpr = DTN.get<CXXThisExpr>()) {
+    if (ThisExpr->isImplicit())
+      EndLoc = BeginLoc;
+  }
+  return {SrcMgr.getExpansionLoc(BeginLoc), SrcMgr.getExpansionLoc(EndLoc)};
+}
+
 namespace {
 /// Counts the number of nodes that will be compared.
 struct NodeCountVisitor : public RecursiveASTVisitor<NodeCountVisitor> {
   int Count = 0;
   const SyntaxTree::Impl &Tree;
   NodeCountVisitor(const SyntaxTree::Impl &Tree) : Tree(Tree) {}
   bool TraverseDecl(Decl *D) {
-    if (isNodeExcluded(Tree.AST.getSourceManager(), D))
+    if (isNodeExcluded(Tree.AST, D))
       return true;
     ++Count;
     RecursiveASTVisitor<NodeCountVisitor>::TraverseDecl(D);
     return true;
   }
   bool TraverseStmt(Stmt *S) {
     if (S)
       S = S->IgnoreImplicit();
-    if (isNodeExcluded(Tree.AST.getSourceManager(), S))
+    if (isNodeExcluded(Tree.AST, S))
       return true;
     ++Count;
     RecursiveASTVisitor<NodeCountVisitor>::TraverseStmt(S);
@@ -259,7 +277,7 @@
       N.Height = std::max(N.Height, 1 + Tree.getNode(Child).Height);
   }
   bool TraverseDecl(Decl *D) {
-    if (isNodeExcluded(Tree.AST.getSourceManager(), D))
+    if (isNodeExcluded(Tree.AST, D))
       return true;
     auto SavedState = PreTraverse(D);
     RecursiveASTVisitor<PreorderVisitor>::TraverseDecl(D);
@@ -269,7 +287,7 @@
   bool TraverseStmt(Stmt *S) {
     if (S)
       S = S->IgnoreImplicit();
-    if (isNodeExcluded(Tree.AST.getSourceManager(), S))
+    if (isNodeExcluded(Tree.AST, S))
       return true;
     auto SavedState = PreTraverse(S);
     RecursiveASTVisitor<PreorderVisitor>::TraverseStmt(S);
@@ -280,7 +298,7 @@
 };
 } // end anonymous namespace
 
-SyntaxTree::Impl::Impl(SyntaxTree *Parent, Decl *N, ASTContext &AST)
+SyntaxTree::Impl::Impl(SyntaxTree *Parent, Decl *N, ASTUnit &AST)
     : Parent(Parent), AST(AST) {
   NodeCountVisitor NodeCounter(*this);
   NodeCounter.TraverseDecl(N);
@@ -290,7 +308,7 @@
   initTree();
 }
 
-SyntaxTree::Impl::Impl(SyntaxTree *Parent, Stmt *N, ASTContext &AST)
+SyntaxTree::Impl::Impl(SyntaxTree *Parent, Stmt *N, ASTUnit &AST)
     : Parent(Parent), AST(AST) {
   NodeCountVisitor NodeCounter(*this);
   NodeCounter.TraverseStmt(N);
@@ -403,10 +421,9 @@
   return getRelativeName(ND, ND->getDeclContext());
 }
 
-static const DeclContext *getEnclosingDeclContext(ASTContext &AST,
-                                                  const Stmt *S) {
+static const DeclContext *getEnclosingDeclContext(ASTUnit &AST, const Stmt *S) {
   while (S) {
-    const auto &Parents = AST.getParents(*S);
+    const auto &Parents = AST.getASTContext().getParents(*S);
     if (Parents.empty())
       return nullptr;
     const auto &P = Parents[0];
@@ -423,6 +440,11 @@
 
 std::string SyntaxTree::Impl::getNodeValue(const Node &N) const {
   const DynTypedNode &DTN = N.ASTNode;
+  if (N.isMacro()) {
+    CharSourceRange Range(getSourceRange(AST, N.ASTNode), false);
+    return Lexer::getSourceText(Range, AST.getSourceManager(),
+                                AST.getLangOpts());
+  }
   if (auto *S = DTN.get<Stmt>())
     return getStmtValue(S);
   if (auto *D = DTN.get<Decl>())
@@ -718,9 +740,19 @@
   return ASTNode.getNodeKind();
 }
 
-StringRef Node::getTypeLabel() const { return getType().asStringRef(); }
+StringRef Node::getTypeLabel() const {
+  if (isMacro())
+    return "Macro";
+  return getType().asStringRef();
+}
+
+bool Node::isMacro() const {
+  return ASTNode.getSourceRange().getBegin().isMacroID();
+}
 
 llvm::Optional<std::string> Node::getQualifiedIdentifier() const {
+  if (isMacro())
+    return llvm::None;
   if (auto *ND = ASTNode.get<NamedDecl>()) {
     if (ND->getDeclName().isIdentifier())
       return ND->getQualifiedNameAsString();
@@ -731,6 +763,8 @@
 }
 
 llvm::Optional<StringRef> Node::getIdentifier() const {
+  if (isMacro())
+    return llvm::None;
   if (auto *ND = ASTNode.get<NamedDecl>()) {
     if (ND->getDeclName().isIdentifier())
       return ND->getName();
@@ -1059,13 +1093,17 @@
   return DiffImpl->getMapped(SourceTree.TreeImpl, Id);
 }
 
-SyntaxTree::SyntaxTree(ASTContext &AST)
+SyntaxTree::SyntaxTree(ASTUnit &AST)
     : TreeImpl(llvm::make_unique<SyntaxTree::Impl>(
-          this, AST.getTranslationUnitDecl(), AST)) {}
+          this, AST.getASTContext().getTranslationUnitDecl(), AST)) {}
 
 SyntaxTree::~SyntaxTree() = default;
 
-const ASTContext &SyntaxTree::getASTContext() const { return TreeImpl->AST; }
+ASTUnit &SyntaxTree::getASTUnit() const { return TreeImpl->AST; }
+
+const ASTContext &SyntaxTree::getASTContext() const {
+  return TreeImpl->AST.getASTContext();
+}
 
 const Node &SyntaxTree::getNode(NodeId Id) const {
   return TreeImpl->getNode(Id);
@@ -1085,16 +1123,9 @@
 std::pair<unsigned, unsigned>
 SyntaxTree::getSourceRangeOffsets(const Node &N) const {
   const SourceManager &SrcMgr = TreeImpl->AST.getSourceManager();
-  SourceRange Range = N.ASTNode.getSourceRange();
-  SourceLocation BeginLoc = Range.getBegin();
-  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
-      Range.getEnd(), /*Offset=*/0, SrcMgr, TreeImpl->AST.getLangOpts());
-  if (auto *ThisExpr = N.ASTNode.get<CXXThisExpr>()) {
-    if (ThisExpr->isImplicit())
-      EndLoc = BeginLoc;
-  }
-  unsigned Begin = SrcMgr.getFileOffset(SrcMgr.getExpansionLoc(BeginLoc));
-  unsigned End = SrcMgr.getFileOffset(SrcMgr.getExpansionLoc(EndLoc));
+  SourceRange Range = getSourceRange(TreeImpl->AST, N.ASTNode);
+  unsigned Begin = SrcMgr.getFileOffset(Range.getBegin());
+  unsigned End = SrcMgr.getFileOffset(Range.getEnd());
   return {Begin, End};
 }
 
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===================================================================
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -20,6 +20,7 @@
 #ifndef LLVM_CLANG_TOOLING_ASTDIFF_ASTDIFF_H
 #define LLVM_CLANG_TOOLING_ASTDIFF_ASTDIFF_H
 
+#include "clang/Frontend/ASTUnit.h"
 #include "clang/Tooling/ASTDiff/ASTDiffInternal.h"
 
 namespace clang {
@@ -45,6 +46,7 @@
   ast_type_traits::ASTNodeKind getType() const;
   StringRef getTypeLabel() const;
   bool isLeaf() const { return Children.empty(); }
+  bool isMacro() const;
   llvm::Optional<StringRef> getIdentifier() const;
   llvm::Optional<std::string> getQualifiedIdentifier() const;
 };
@@ -68,14 +70,15 @@
 class SyntaxTree {
 public:
   /// Constructs a tree from a translation unit.
-  SyntaxTree(ASTContext &AST);
+  SyntaxTree(ASTUnit &AST);
   /// Constructs a tree from any AST node.
   template <class T>
-  SyntaxTree(T *Node, ASTContext &AST)
+  SyntaxTree(T *Node, ASTUnit &AST)
       : TreeImpl(llvm::make_unique<Impl>(this, Node, AST)) {}
   SyntaxTree(SyntaxTree &&Other) = default;
   ~SyntaxTree();
 
+  ASTUnit &getASTUnit() const;
   const ASTContext &getASTContext() const;
   StringRef getFilename() const;
 
@@ -88,7 +91,10 @@
   const Node &getNode(NodeId Id) const;
   int findPositionInParent(NodeId Id) const;
 
-  // Returns the starting and ending offset of the node in its source file.
+  /// Returns the range that contains the text that is associated with this
+  /// node.
+  /* SourceRange getSourceRange(const Node &N) const; */
+  /// Returns the offsets for the range returned by getSourceRange.
   std::pair<unsigned, unsigned> getSourceRangeOffsets(const Node &N) const;
 
   /// Serialize the node attributes to a string representation. This should
@@ -118,7 +124,7 @@
 
   /// Returns false if the nodes should never be matched.
   bool isMatchingAllowed(const Node &N1, const Node &N2) const {
-    return N1.getType().isSame(N2.getType());
+    return (N1.isMacro() && N2.isMacro()) || N1.getType().isSame(N2.getType());
   }
 };
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to