[clang-diff] Refactor stop-after command-line flag

[jgravelle-google]

Summary

Rename stop-after to stop-after-topdown. When building LLVM with
-DLLVM_BUILD_LLVM_DYLIB=ON, stop-after collides with the stop-after
already present in LLVM.

This also refactors the flag to a boolean, to reflect that it can only
be in two states.

[johannes]

Looks good, thanks!
You can remove StopAfterTopDown from the ComparisonOptions struct definition

I have a second option stop-after option coming up but I will use another boolean for it, it is simpler this way.

[johannes]

Oh wait I get linker errors with BUILD_SHARED_LIBS=1

undefined reference to ClangDiffCategory in ASTDiff

[jgravelle-google]

If you have more stop-after options it'd probably be simpler to leave it as one field. I'll just rename it to "stop-diff-after" to avoid the name collision.

[jgravelle-google]

• Undo refactor, just change name

[johannes]

In D36989#848835, @jgravelle-google wrote:

If you have more stop-after options it'd probably be simpler to leave it as one field. I'll just rename it to "stop-diff-after" to avoid the name collision.

Ok, you can go ahead. Or, if I should do it let me know.

lib/Tooling/ASTDiff/ASTDiff.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Tooling/ASTDiff/ASTDiff.h"
 
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/PriorityQueue.h"
+#include "llvm/Support/CommandLine.h"
 
 #include <limits>
 #include <memory>
 #include <unordered_set>
 
 using namespace llvm;
 using namespace clang;
 
+extern cl::OptionCategory ClangDiffCategory;
+static cl::opt<bool> StopAfterTopDown("stop-after-topdown",
+                                      cl::desc("Stops after top-down matching"),
+                                      cl::Optional, cl::init(false),
+                                      cl::cat(ClangDiffCategory));
+
 namespace clang {
 namespace diff {
 
 namespace {
 /// Maps nodes of the left tree to ones on the right, and vice versa.
 class Mapping {
 public:
   Mapping() = default;
ASTDiff::Impl::Impl(SyntaxTree::Impl &T1, SyntaxTree::Impl &T2,
                     const ComparisonOptions &Options)
     : T1(T1), T2(T2), Options(Options) {
   computeMapping();
   computeChangeKinds(TheMapping);
 }
 
 void ASTDiff::Impl::computeMapping() {
   TheMapping = matchTopDown();
-  if (Options.StopAfterTopDown)
+  if (StopAfterTopDown)
     return;
   matchBottomUp(TheMapping);
 }
 
 void ASTDiff::Impl::computeChangeKinds(Mapping &M) {
   for (NodeId Id1 : T1) {
     if (!M.hasSrc(Id1)) {
       T1.getMutableNode(Id1).Change = Delete;

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 -- -std=c++11 | 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.

tools/clang-diff/ClangDiff.cpp

 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/CommandLine.h"
 
 using namespace llvm;
 using namespace clang;
 using namespace clang::tooling;
 
-static cl::OptionCategory ClangDiffCategory("clang-diff options");
+cl::OptionCategory ClangDiffCategory("clang-diff options");
 
 static cl::opt<bool>
     ASTDump("ast-dump",
             cl::desc("Print the internal representation of the AST."),
             cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt<bool> ASTDumpJson(
     "ast-dump-json",
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 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",
int main(int argc, const char **argv) {
   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\n";
-      return 1;
-    }
-  }
   diff::SyntaxTree SrcTree(Src->getASTContext());
   diff::SyntaxTree DstTree(Dst->getASTContext());
   diff::ASTDiff Diff(SrcTree, DstTree, Options);
 
   if (HtmlDiff) {
     llvm::outs() << HtmlDiffHeader << "<pre>";
     llvm::outs() << "<div id='L' class='code'>";
     printHtmlForNode(llvm::outs(), Diff, SrcTree, true, SrcTree.getRootId(), 0);