[clang-diff] Add commandline arguments.

[johannes]

Summary

Support command line options for build path and extra arguments
This emulates the options accepted by clang tools that use CommonOptionsParser.

Add a flag to stop after top down matching.

Add a flag for controlling the maximum size parameter for bottom up matching.

[arphaman]

There should be a test that ensures that stop-after works and extra-args are correctly passed to the compiler.

[johannes]

add some test, replace -no-compilation-database with --

[johannes]

In D36177#828909, @arphaman wrote:

There should be a test that ensures that stop-after works and extra-args are correctly passed to the compiler.

stop-after is not tested yet. It is also broken in this patch, I will move it to a later patch, then it is easier to test.

[arphaman]

test/Tooling/clang-diff-args.sh

1

I think a '.test' extension rather than '.sh' is better

3

You can just have a CHECK in this file and run FileCheck %s

[johannes]

fix tests

[arphaman]

I think you have to test the -p option too (check the compilation database can be loaded and arguments are propagated to the parser), otherwise LGTM!

include/clang/Tooling/ASTDiff/ASTDiff.h

struct ComparisonOptions {
   /// Whenever two subtrees are matched in the bottom-up phase, the optimal
   /// mapping is computed, unless the size of either subtrees exceeds this.
   int MaxSize = 100;
 
   /// If this is set to true, nodes that have parents that must not be matched
   /// (see NodeComparison) will be allowed to be matched.
   bool EnableMatchingWithUnmatchableParents = false;
 
+  bool StopAfterTopDown = false;
+
   /// Returns false if the nodes should never be matched.
   bool isMatchingAllowed(const DynTypedNode &N1, const DynTypedNode &N2) const {
     return N1.getNodeKind().isSame(N2.getNodeKind());
   }
 };
 
 } // end namespace diff
 } // end namespace clang
 
 #endif

lib/Tooling/ASTDiff/ASTDiff.cpp

Mapping ASTDiff::Impl::matchTopDown() const {
   }
   return M;
 }
 
 void ASTDiff::Impl::computeMapping() {
   if (IsMappingDone)
     return;
   TheMapping = matchTopDown();
+  if (Options.StopAfterTopDown)
+    return;
   matchBottomUp(TheMapping);
   IsMappingDone = true;
 }
 
 std::vector<Match> ASTDiff::Impl::getMatches(Mapping &M) {
   std::vector<Match> Matches;
   for (NodeId Id1 = 0, Id2, E = T1.getSize(); Id1 < E; ++Id1)
     if ((Id2 = M.getDst(Id1)).isValid())

test/Tooling/clang-diff-args.sh

+RUN: echo a > %t.cpp
+
+RUN: echo "CHECK: unknown type name 'X'" > %t1
+
+check adding command-line arguments
+RUN: clang-diff -ast-dump -extra-arg=-Da=X        %t.cpp -- 2>&1 | FileCheck %t1
+RUN: clang-diff -ast-dump -extra-arg-before=-Da=X %t.cpp -- 2>&1 | FileCheck %t1
+RUN: clang-diff -ast-dump %t.cpp -- 2>&1 -Da=X | FileCheck %t1

test/Tooling/clang-diff-basic.cpp

 // RUN: %clang_cc1 -E %s > %T/src.cpp
 // RUN: %clang_cc1 -E %s > %T/dst.cpp -DDEST
-// RUN: clang-diff -no-compilation-database %T/src.cpp %T/dst.cpp | FileCheck %s
+// RUN: clang-diff %T/src.cpp %T/dst.cpp -- | FileCheck %s
 
 #ifndef DEST
 namespace src {
 
 void foo() {
   int x = 321;
 }
 

tools/clang-diff/ClangDiff.cpp

 
 static cl::OptionCategory ClangDiffCategory("clang-diff options");
 
 static cl::opt<bool>
     ASTDump("ast-dump",
             cl::desc("Print the internal representation of the AST as JSON."),
             cl::init(false), cl::cat(ClangDiffCategory));
 
-static cl::opt<bool> NoCompilationDatabase(
-    "no-compilation-database",
-    cl::desc(
-        "Do not attempt to load build settings from a compilation database"),
-    cl::init(false), cl::cat(ClangDiffCategory));
-
 static cl::opt<std::string> SourcePath(cl::Positional, cl::desc("<source>"),
                                        cl::Required,
                                        cl::cat(ClangDiffCategory));
 
 static cl::opt<std::string> DestinationPath(cl::Positional,
                                             cl::desc("<destination>"),
                                             cl::Optional,
                                             cl::cat(ClangDiffCategory));
 
-static std::unique_ptr<ASTUnit> getAST(const StringRef Filename) {
+static cl::opt<std::string> StopAfter("stop-after",
+                                      cl::desc("<topdown|bottomup>"),
+                                      cl::Optional, cl::init(""),
+                                      cl::cat(ClangDiffCategory));
+
+static cl::opt<int> MaxSize("s", cl::desc("<maxsize>"), cl::Optional,
+                            cl::init(-1), cl::cat(ClangDiffCategory));
+
+static cl::opt<std::string> BuildPath("p", cl::desc("Build path"), cl::init(""),
+                                      cl::Optional, cl::cat(ClangDiffCategory));
+
+static cl::list<std::string> ArgsAfter(
+    "extra-arg",
+    cl::desc("Additional argument to append to the compiler command line"),
+    cl::cat(ClangDiffCategory));
+
+static cl::list<std::string> ArgsBefore(
+    "extra-arg-before",
+    cl::desc("Additional argument to prepend to the compiler command line"),
+    cl::cat(ClangDiffCategory));
+
+static void addExtraArgs(std::unique_ptr<CompilationDatabase> &Compilations) {
+  if (!Compilations)
+    return;
+  auto AdjustingCompilations =
+      llvm::make_unique<ArgumentsAdjustingCompilations>(
+          std::move(Compilations));
+  AdjustingCompilations->appendArgumentsAdjuster(
+      getInsertArgumentAdjuster(ArgsBefore, ArgumentInsertPosition::BEGIN));
+  AdjustingCompilations->appendArgumentsAdjuster(
+      getInsertArgumentAdjuster(ArgsAfter, ArgumentInsertPosition::END));
+  Compilations = std::move(AdjustingCompilations);
+}
+
+static std::unique_ptr<ASTUnit>
+getAST(const std::unique_ptr<CompilationDatabase> &CommonCompilations,
+       const StringRef Filename) {
   std::string ErrorMessage;
   std::unique_ptr<CompilationDatabase> Compilations;
-  if (!NoCompilationDatabase)
-    Compilations =
-        CompilationDatabase::autoDetectFromSource(Filename, ErrorMessage);
+  if (!CommonCompilations) {
+    Compilations = CompilationDatabase::autoDetectFromSource(
+        BuildPath.empty() ? Filename : BuildPath, ErrorMessage);
     if (!Compilations) {
-    if (!NoCompilationDatabase)
       llvm::errs()
           << "Error while trying to load a compilation database, running "
              "without flags.\n"
           << ErrorMessage;
-    Compilations = llvm::make_unique<clang::tooling::FixedCompilationDatabase>(
+      Compilations =
+          llvm::make_unique<clang::tooling::FixedCompilationDatabase>(
               ".", std::vector<std::string>());
     }
+  }
+  addExtraArgs(Compilations);
   std::array<std::string, 1> Files = {{Filename}};
-  ClangTool Tool(*Compilations, Files);
+  ClangTool Tool(Compilations ? *Compilations : *CommonCompilations, Files);
   std::vector<std::unique_ptr<ASTUnit>> ASTs;
   Tool.buildASTs(ASTs);
   if (ASTs.size() != Files.size())
     return nullptr;
   return std::move(ASTs[0]);
 }
 
 int main(int argc, const char **argv) {
+  std::string ErrorMessage;
+  std::unique_ptr<CompilationDatabase> CommonCompilations =
+      FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage);
+  if (!CommonCompilations && !ErrorMessage.empty())
+    llvm::errs() << ErrorMessage;
   cl::HideUnrelatedOptions(ClangDiffCategory);
   if (!cl::ParseCommandLineOptions(argc, argv)) {
     cl::PrintOptionValues();
     return 1;
   }
 
+  addExtraArgs(CommonCompilations);
+
   if (ASTDump) {
     if (!DestinationPath.empty()) {
       llvm::errs() << "Error: Please specify exactly one filename.\n";
       return 1;
     }
-    std::unique_ptr<ASTUnit> AST = getAST(SourcePath);
+    std::unique_ptr<ASTUnit> AST = getAST(CommonCompilations, SourcePath);
     if (!AST)
       return 1;
     diff::SyntaxTree Tree(AST->getASTContext());
     Tree.printAsJson(llvm::outs());
     return 0;
   }
 
   if (DestinationPath.empty()) {
     llvm::errs() << "Error: Exactly two paths are required.\n";
     return 1;
   }
 
-  std::unique_ptr<ASTUnit> Src = getAST(SourcePath);
-  std::unique_ptr<ASTUnit> Dst = getAST(DestinationPath);
+  std::unique_ptr<ASTUnit> Src = getAST(CommonCompilations, SourcePath);
+  std::unique_ptr<ASTUnit> Dst = getAST(CommonCompilations, DestinationPath);
   if (!Src || !Dst)
     return 1;
 
   diff::ComparisonOptions Options;
+  if (MaxSize != -1)
+    Options.MaxSize = MaxSize;
+  if (!StopAfter.empty()) {
+    if (StopAfter == "topdown")
+      Options.StopAfterTopDown = true;
+    else if (StopAfter != "bottomup") {
+      llvm::errs() << "Error: Invalid argument for -stop-after";
+      return 1;
+    }
+  }
   diff::SyntaxTree SrcTree(Src->getASTContext());
   diff::SyntaxTree DstTree(Dst->getASTContext());
   diff::ASTDiff DiffTool(SrcTree, DstTree, Options);
   for (const auto &Match : DiffTool.getMatches())
     DiffTool.printMatch(llvm::outs(), Match);
   for (const auto &Change : DiffTool.getChanges())
     DiffTool.printChange(llvm::outs(), Change);
 
   return 0;
 }