[clang-query] Add option to enable only displaying main file matches

[njames93]

Summary

Add option set match-scope (include-headers|main-file-only) to control only printing matches from the main file.
This can be set once and then avoids the need to pollute all your match expressions with isExpansionInMainFile()
By default it is enabled to only show match results from the main file.

[aaron.ballman]

clang-tools-extra/clang-query/Query.cpp

58–59

I'm not super keen on this being the default behavior. One of the big uses for clang-query is to experiment with matchers and I think ignoring nodes by default makes it harder to know whether you've written the matcher correctly or not. e.g., you write the matcher, run it over a source file, don't get any results -- is that because the matcher was nonsense or because the mode excluded matching on nodes in headers by default?

The fact that we print how many nodes were skipped does help (so it's not a silent when you match only in headers), but I think the default behavior should be to match on what's in the source code (which includes header contents) and users should have to opt into getting less results. WDYT?

[njames93]

clang-tools-extra/clang-query/Query.cpp

58–59

I thought of making main-file-only the default as its the behaviour that clang-tidy exhibits by default. That's also the reason I included printing details of skipped matching nodes. But you're right there's definitely a compelling argument to leave the default as matching from header files as well.

[njames93]

Change default match mode to include all files.
Update so the prompt to switch match mode is only displayed once.

[aaron.ballman]

LGTM aside from a typo fix.

clang-tools-extra/clang-query/Query.cpp

56

[njames93]

Fix typo

clang-tools-extra/clang-query/Query.cpp

OS << "Available commands:\n\n"
         "Set whether to print the current matcher,\n"
         "  set traversal <kind>              "
         "Set traversal kind of clang-query session. Available kinds are:\n"
         "    AsIs                            "
         "Print and match the AST as clang sees it.  This mode is the "
         "default.\n"
         "    IgnoreUnlessSpelledInSource     "
         "Omit AST nodes unless spelled in the source.\n"
+        "  set match-scope <kind>            "
+        "Set the scope of matchers. Available kinds are:\n"
+        "    include-headers                 "
+        "Match nodes included from header files. This mode is the "
+        "default.\n"
+        "    main-file-only                  "
+        "Only match nodes from the main source file.\n"
         "  set output <feature>              "
         "Set whether to output only <feature> content.\n"
         "  enable output <feature>           "
         "Enable <feature> content non-exclusively.\n"
         "  disable output <feature>          "
         "Disable <feature> content non-exclusively.\n"
         "  quit, q                           "
         "Terminates the query session.\n\n"
struct CollectBoundNodes : MatchFinder::MatchCallback {
   CollectBoundNodes(std::vector<BoundNodes> &Bindings) : Bindings(Bindings) {}
   void run(const MatchFinder::MatchResult &Result) override {
     Bindings.push_back(Result.Nodes);
   }
 };
 
 } // namespace
 
+static void printMatch(llvm::raw_ostream &OS, QuerySession &QS, ASTUnit &AST,
+                       const DynTypedNode &Node, StringRef BindName) {
+  if (QS.DiagOutput) {
+    clang::SourceRange R = Node.getSourceRange();
+    if (R.isValid()) {
+      TextDiagnostic TD(OS, AST.getASTContext().getLangOpts(),
+                        &AST.getDiagnostics().getDiagnosticOptions());
+      TD.emitDiagnostic(FullSourceLoc(R.getBegin(), AST.getSourceManager()),
+                        DiagnosticsEngine::Note,
+                        ("\"" + BindName + "\" binds here").str(),
+                        CharSourceRange::getTokenRange(R), None);
+    }
+  }
+  if (QS.PrintOutput) {
+    OS << "Binding for \"" << BindName << "\":\n";
+    Node.print(OS, AST.getASTContext().getPrintingPolicy());
+    OS << "\n";
+  }
+  if (QS.DetailedASTOutput) {
+    OS << "Binding for \"" << BindName << "\":\n";
+    const ASTContext &Ctx = AST.getASTContext();
+    ASTDumper Dumper(OS, Ctx, AST.getDiagnostics().getShowColors());
+    Dumper.SetTraversalKind(QS.TK);
+    Dumper.Visit(Node);
+    OS << "\n";
+  }
+}
+
 bool MatchQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
   unsigned MatchCount = 0;
+  unsigned MatchesFromHeaders = 0;
 
   for (auto &AST : QS.ASTs) {
     MatchFinder Finder;
     std::vector<BoundNodes> Matches;
-    DynTypedMatcher MaybeBoundMatcher = Matcher;
-    if (QS.BindRoot) {
-      llvm::Optional<DynTypedMatcher> M = Matcher.tryBind("root");
-      if (M)
-        MaybeBoundMatcher = *M;
-    }
     CollectBoundNodes Collect(Matches);
-    if (!Finder.addDynamicMatcher(MaybeBoundMatcher, &Collect)) {
+    if (!Finder.addDynamicMatcher(Matcher, &Collect)) {
       OS << "Not a valid top-level matcher.\n";
       return false;
     }
 
     AST->getASTContext().getParentMapContext().setTraversalKind(QS.TK);
     Finder.matchAST(AST->getASTContext());
 
     if (QS.PrintMatcher) {
if (QS.PrintMatcher) {
         OS << "\n" << std::string(PrefixText.size() + 2, ' ') << Line;
         MaxLength = std::max<int>(MaxLength, Line.rtrim().size());
       }
 
       OS << "\n"
          << "  " << std::string(PrefixText.size() + MaxLength, '=') << "\n\n";
     }
 
-    for (auto MI = Matches.begin(), ME = Matches.end(); MI != ME; ++MI) {
-      OS << "\nMatch #" << ++MatchCount << ":\n\n";
-
-      for (auto BI = MI->getMap().begin(), BE = MI->getMap().end(); BI != BE;
-           ++BI) {
-        if (QS.DiagOutput) {
-          clang::SourceRange R = BI->second.getSourceRange();
-          if (R.isValid()) {
-            TextDiagnostic TD(OS, AST->getASTContext().getLangOpts(),
-                              &AST->getDiagnostics().getDiagnosticOptions());
-            TD.emitDiagnostic(
-                FullSourceLoc(R.getBegin(), AST->getSourceManager()),
-                DiagnosticsEngine::Note, "\"" + BI->first + "\" binds here",
-                CharSourceRange::getTokenRange(R), None);
-          }
+    for (const auto &Match : Matches) {
+      const DynTypedNode &RootNode = Match.getRoot();
+      if (QS.OnlyMainFileMatches || LLVM_UNLIKELY(QS.HeaderModeWarnOnce)) {
+        SourceRange SR = RootNode.getSourceRange();
+        if (SR.isValid()) {
+          const SourceManager &SM = AST->getSourceManager();
+          if (!SM.isInMainFile(SM.getExpansionLoc(SR.getBegin())) ||
+              !SM.isInMainFile(SM.getExpansionLoc(SR.getEnd()))) {
+            ++MatchesFromHeaders;
+            if (LLVM_LIKELY(QS.OnlyMainFileMatches))
+              continue;
           }
-        if (QS.PrintOutput) {
-          OS << "Binding for \"" << BI->first << "\":\n";
-          BI->second.print(OS, AST->getASTContext().getPrintingPolicy());
-          OS << "\n";
         }
-        if (QS.DetailedASTOutput) {
-          OS << "Binding for \"" << BI->first << "\":\n";
-          const ASTContext &Ctx = AST->getASTContext();
-          ASTDumper Dumper(OS, Ctx, AST->getDiagnostics().getShowColors());
-          Dumper.SetTraversalKind(QS.TK);
-          Dumper.Visit(BI->second);
-          OS << "\n";
-        }
+      OS << "\nMatch #" << ++MatchCount << ":\n\n";
+
+      for (const auto &NameAndNode : Match.getMap()) {
+        printMatch(OS, QS, *AST, NameAndNode.second, NameAndNode.first);
       }
 
-      if (MI->getMap().empty())
+      if (QS.BindRoot)
+        printMatch(OS, QS, *AST, RootNode, "root");
+      else if (Match.getMap().empty())
         OS << "No bindings.\n";
     }
   }
 
   OS << MatchCount << (MatchCount == 1 ? " match.\n" : " matches.\n");
+  if (MatchesFromHeaders > 0) {
+    if (QS.OnlyMainFileMatches) {
+      OS << "Skipped " << MatchesFromHeaders
+         << (MatchesFromHeaders == 1 ? " match" : " matches")
+         << " from header files.\n";
+      if (QS.HeaderModeWarnOnce)
+        OS << "Use 'set match-mode include-headers' to display them.\n";
+    } else {
+      assert(QS.HeaderModeWarnOnce);
+      OS << "Included " << MatchesFromHeaders
+         << (MatchesFromHeaders == 1 ? " match" : " matches")
+         << " from header files.\nUse 'set match-mode main-file-only' to hide "
+            "them.\n";
+    }
+    QS.HeaderModeWarnOnce = false;
+  }
   return true;
 }
 
 bool LetQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
   if (Value) {
     QS.NamedValues[Name] = Value;
   } else {
     QS.NamedValues.erase(Name);

clang-tools-extra/clang-query/QueryParser.h

QueryParser(StringRef Line, const QuerySession &QS)
       : Line(Line), CompletionPos(nullptr), QS(QS) {}
 
   StringRef lexWord();
 
   template <typename T> struct LexOrCompleteWord;
 
   QueryRef parseSetBool(bool QuerySession::*Var);
   QueryRef parseSetTraversalKind(TraversalKind QuerySession::*Var);
+  QueryRef parseMatchScope(bool QuerySession::*Var);
   template <typename QueryType> QueryRef parseSetOutputKind();
   QueryRef completeMatcherExpression();
 
   QueryRef endQuery(QueryRef Q);
 
   /// Parse [\p Begin,\p End).
   ///
   /// \return A reference to the parsed query object, which may be an

clang-tools-extra/clang-query/QueryParser.cpp

unsigned Value =
           .Case("IgnoreUnlessSpelledInSource", TK_IgnoreUnlessSpelledInSource)
           .Default(~0u);
   if (Value == ~0u) {
     return new InvalidQuery("expected traversal kind, got '" + ValStr + "'");
   }
   return new SetQuery<TraversalKind>(Var, static_cast<TraversalKind>(Value));
 }
 
+QueryRef QueryParser::parseMatchScope(bool QuerySession::*Var) {
+  StringRef ValStr;
+  unsigned Value = LexOrCompleteWord<unsigned>(this, ValStr)
+                       .Case("main-file-only", 1)
+                       .Case("include-headers", 0)
+                       .Default(~0U);
+  if (Value == ~0U) {
+    return new InvalidQuery("expected match scope, got '" + ValStr + "'");
+  }
+  return new SetQuery<bool>(Var, Value);
+}
+
 QueryRef QueryParser::endQuery(QueryRef Q) {
   StringRef Extra = Line;
   StringRef ExtraTrimmed = Extra.drop_while(
       [](char c) { return StringRef(" \t\v\f\r").contains(c); });
 
   if ((!ExtraTrimmed.empty() && ExtraTrimmed[0] == '\n') ||
       (ExtraTrimmed.size() >= 2 && ExtraTrimmed[0] == '\r' &&
        ExtraTrimmed[1] == '\n'))
enum ParsedQueryKind {
   PQK_Disable
 };
 
 enum ParsedQueryVariable {
   PQV_Invalid,
   PQV_Output,
   PQV_BindRoot,
   PQV_PrintMatcher,
-  PQV_Traversal
+  PQV_Traversal,
+  PQV_OnlyMainFileMatches
 };
 
 QueryRef makeInvalidQueryFromDiagnostics(const Diagnostics &Diag) {
   std::string ErrStr;
   llvm::raw_string_ostream OS(ErrStr);
   Diag.printToStreamFull(OS);
   return new InvalidQuery(OS.str());
 }
QueryRef QueryParser::doParse() {
   case PQK_Set: {
     StringRef VarStr;
     ParsedQueryVariable Var =
         LexOrCompleteWord<ParsedQueryVariable>(this, VarStr)
             .Case("output", PQV_Output)
             .Case("bind-root", PQV_BindRoot)
             .Case("print-matcher", PQV_PrintMatcher)
             .Case("traversal", PQV_Traversal)
+            .Case("match-scope", PQV_OnlyMainFileMatches)
             .Default(PQV_Invalid);
     if (VarStr.empty())
       return new InvalidQuery("expected variable name");
     if (Var == PQV_Invalid)
       return new InvalidQuery("unknown variable: '" + VarStr + "'");
 
     QueryRef Q;
     switch (Var) {
     case PQV_Output:
       Q = parseSetOutputKind<SetExclusiveOutputQuery>();
       break;
     case PQV_BindRoot:
       Q = parseSetBool(&QuerySession::BindRoot);
       break;
     case PQV_PrintMatcher:
       Q = parseSetBool(&QuerySession::PrintMatcher);
       break;
     case PQV_Traversal:
       Q = parseSetTraversalKind(&QuerySession::TK);
       break;
+    case PQV_OnlyMainFileMatches:
+      Q = parseMatchScope(&QuerySession::OnlyMainFileMatches);
+      break;
     case PQV_Invalid:
       llvm_unreachable("Invalid query kind");
     }
 
     return endQuery(Q);
   }
   case PQK_Enable:
   case PQK_Disable: {

clang-tools-extra/clang-query/QuerySession.h

 namespace query {
 
 /// Represents the state for a particular clang-query session.
 class QuerySession {
 public:
   QuerySession(llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs)
       : ASTs(ASTs), PrintOutput(false), DiagOutput(true),
         DetailedASTOutput(false), BindRoot(true), PrintMatcher(false),
-        Terminate(false), TK(TK_AsIs) {}
+        OnlyMainFileMatches(false), Terminate(false), HeaderModeWarnOnce(true),
+        TK(TK_AsIs) {}
 
   llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs;
 
   bool PrintOutput;
   bool DiagOutput;
   bool DetailedASTOutput;
 
   bool BindRoot;
   bool PrintMatcher;
+  bool OnlyMainFileMatches;
   bool Terminate;
 
+  // Use this to print the set match-scope help once.
+  bool HeaderModeWarnOnce;
+
   TraversalKind TK;
   llvm::StringMap<ast_matchers::dynamic::VariantValue> NamedValues;
 };
 
 } // namespace query
 } // namespace clang
 
 #endif

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clang-doc
 -------------------------
 
 The improvements are...
 
 Improvements to clang-query
 ---------------------------
 
-The improvements are...
+ - Added a option `set match-scope` to control displaying matches that occur 
+   in header files.
 
 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------

clang-tools-extra/test/clang-query/Inputs/foo.h

+#ifndef INPUTS_FOO_HH
+#define INPUTS_FOO_HH
+
+void foo();
+void bar();
+
+#endif // INPUTS_FOO_HH

clang-tools-extra/test/clang-query/headers.cpp

+// RUN: clang-query -c "set match-scope include-headers" -c "match functionDecl(hasName('foo'))" %s -- | FileCheck %s --check-prefix=CHECK-ALL-FOO
+// RUN: clang-query -c "set match-scope main-file-only" -c "match functionDecl(hasName('foo'))" %s -- | FileCheck %s --check-prefix=CHECK-MAIN-FOO
+// RUN: clang-query -c "set match-scope include-headers" -c "match functionDecl(hasName('bar'))" %s -- | FileCheck %s --check-prefix=CHECK-ALL-BAR
+// RUN: clang-query -c "set match-scope main-file-only" -c "match functionDecl(hasName('bar'))" %s -- | FileCheck %s --check-prefix=CHECK-MAIN-BAR
+// RUN: clang-query -c "set match-scope include-headers" -c "match functionDecl()" %s -- | FileCheck %s --check-prefix=CHECK-ALL-ANY
+// RUN: clang-query -c "set match-scope main-file-only" -c "match functionDecl()" %s -- | FileCheck %s --check-prefix=CHECK-MAIN-ANY
+
+// Test to ensure default behavious only includes main file matches.
+// RUN: clang-query -c "match functionDecl()" %s -- | FileCheck %s --check-prefix=CHECK-ALL-ANY
+
+// Test to ensure the warnings about selecting match-scope is only shown once
+// RUN: clang-query -c "set match-scope include-headers" -c "match functionDecl()" -c "match functionDecl()" %s | FileCheck %s --check-prefix=CHECK-SINGLE-WARN-ALL
+// RUN: clang-query -c "set match-scope main-file-only" -c "match functionDecl()" -c "match functionDecl()" %s | FileCheck %s --check-prefix=CHECK-SINGLE-WARN-MAIN
+
+#include "Inputs/foo.h"
+
+void foo() {}
+
+// CHECK-ALL-FOO: /Inputs/foo.h:4:1: note: "root" binds here
+// CHECK-ALL-FOO: /headers.cpp:17:1: note: "root" binds here
+// CHECK-ALL-FOO: 2 matches.
+// CHECK-ALL-FOO-NEXT: Included 1 match from header files
+// CHECK-ALL-FOO-NEXT: Use 'set match-mode main-file-only' to hide them.
+
+// CHECK-MAIN-FOO: /headers.cpp:17:1: note: "root" binds here
+// CHECK-MAIN-FOO: 1 match.
+// CHECK-MAIN-FOO-Next: Skipped 1 match from header files.
+// CHECK-MAIN-FOO-Next: Use 'set match-mode include-headers' to display them.
+
+// CHECK-ALL-BAR: /Inputs/foo.h:5:1: note: "root" binds here
+// CHECK-ALL-BAR: 1 match.
+// CHECK-ALL-BAR-NEXT: Included 1 match from header files
+// CHECK-ALL-BAR-NEXT: Use 'set match-mode main-file-only' to hide them.
+
+// CHECK-MAIN-BAR: 0 matches.
+// CHECK-MAIN-BAR-Next: Skipped 1 match from header files.
+// CHECK-MAIN-BAR-Next: Use 'set match-mode include-headers' to display them.
+
+// CHECK-ALL-ANY: /Inputs/foo.h:4:1: note: "root" binds here
+// CHECK-ALL-ANY: /Inputs/foo.h:5:1: note: "root" binds here
+// CHECK-ALL-ANY: /headers.cpp:17:1: note: "root" binds here
+// CHECK-ALL-ANY: 3 matches.
+// CHECK-ALL-ANY-NEXT: Included 2 matches from header files
+// CHECK-ALL-ANY-NEXT: Use 'set match-mode main-file-only' to hide them.
+
+// CHECK-MAIN-ANY: /headers.cpp:17:1: note: "root" binds here
+// CHECK-MAIN-ANY: 1 match.
+// CHECK-MAIN-ANY-Next: Skipped 2 matches from header files.
+// CHECK-MAIN-ANY-Next: Use 'set match-mode include-headers' to display them.
+
+// CHECK-SINGLE-WARN-ALL: Use 'set match-mode main-file-only' to hide them.
+// CHECK-SINGLE-WARN-ALL-NOT: Use 'set match-mode main-file-only' to hide them.
+
+// CHECK-SINGLE-WARN-MAIN: Use 'set match-mode include-headers' to display them.
+// CHECK-SINGLE-WARN-MAIN-NOT: Use 'set match-mode include-headers' to display them.