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 getIdentifier() const; llvm::Optional 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 - SyntaxTree(T *Node, ASTContext &AST) + SyntaxTree(T *Node, ASTUnit &AST) : TreeImpl(llvm::make_unique(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 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()); } }; 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 Impl(SyntaxTree *Parent, typename std::enable_if::value, T>::type *Node, - ASTContext &AST) + ASTUnit &AST) : Impl(Parent, dyn_cast(Node), AST) {} template Impl(SyntaxTree *Parent, typename std::enable_if::value, T>::type *Node, - ASTContext &AST) + ASTUnit &AST) : Impl(Parent, dyn_cast(Node), AST) {} SyntaxTree *Parent; - ASTContext * + ASTUnit * std::vector Leaves; // Maps preorder indices to postorder ones. std::vector PostorderIds; @@ -173,8 +173,8 @@ static bool isSpecializedNodeExcluded(const Decl *D) { return D->isImplicit(); } static bool isSpecializedNodeExcluded(const Stmt *S) { return false; } -template -static bool isNodeExcluded(const SourceManager &SrcMgr, T *N) { +template static bool isNodeExcluded(ASTUnit &AST, T *N) { + const SourceManager &SrcMgr = AST.getSourceManager(); if (!N) return true; SourceLocation SLoc = N->getLocStart(); @@ -182,13 +182,31 @@ // 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()) { + 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 { @@ -196,7 +214,7 @@ 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::TraverseDecl(D); @@ -205,7 +223,7 @@ bool TraverseStmt(Stmt *S) { if (S) S = S->IgnoreImplicit(); - if (isNodeExcluded(Tree.AST.getSourceManager(), S)) + if (isNodeExcluded(Tree.AST, S)) return true; ++Count; RecursiveASTVisitor::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::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::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()) return getStmtValue(S); if (auto *D = DTN.get()) @@ -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 Node::getQualifiedIdentifier() const { + if (isMacro()) + return llvm::None; if (auto *ND = ASTNode.get()) { if (ND->getDeclName().isIdentifier()) return ND->getQualifiedNameAsString(); @@ -731,6 +763,8 @@ } llvm::Optional Node::getIdentifier() const { + if (isMacro()) + return llvm::None; if (auto *ND = ASTNode.get()) { 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( - 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 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()) { - 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: 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: 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/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: tools/clang-diff/ClangDiff.cpp =================================================================== --- tools/clang-diff/ClangDiff.cpp +++ tools/clang-diff/ClangDiff.cpp @@ -483,7 +483,7 @@ std::unique_ptr 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) {