[clang-query] Continue if compilation command not found for some files

[george.karpenkov]

Summary

This is a somewhat controversial change, but I have found it very convenient when running clang-query on entire projects.

---

When searching for a code pattern in an entire project with a compilation database it's tempting to run

clang-query *.cpp

And yet, that often breaks because some files are just not in the compilation database: tests, sample code, etc.
I don't think clang-query should stop when encountering such cases.

[ilya-biryukov]

The change LG, but maybe also exit with an error code whenever some of the ASTs couldn't be built?

clang-tools-extra/clang-query/tool/ClangQuery.cpp

80

Maybe make the error message scarier, e.g. "Failed to build AST for some of the files. Results may be incomplete."

82

Maybe remove && Status? I don't think it will show up in assertion failure messages anyway.

[george.karpenkov]

@ilya-biryukov Thanks for the review and sorry for the delay :P

clang-tools-extra/clang-query/tool/ClangQuery.cpp

82

Oops, you are right.

[ilya-biryukov]

LGTM with a few NITs.
Also consider adding a test for this.

clang-tools-extra/clang-query/tool/ClangQuery.cpp

76

These error codes are undocumented, so I would probably stick with a single != 0 check.
But up to you.

77

NIT: remove this empty line

123

NIT: Maybe make only a single int with a more descriptive name, e.g. ASTStatus.
The return at the end would be return ASTStatus != 0. But up to you.

[george.karpenkov]

clang-tools-extra/clang-query/tool/ClangQuery.cpp

123

Then why return ASTStatus != 0 and not just return ASTStatus ? :P

[dblaikie]

clang-tools-extra/trunk/clang-query/tool/ClangQuery.cpp

108 ↗

(On Diff #176751)

skip 'else after return' ( https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return )

[ilya-biryukov]

clang-tools-extra/clang-query/tool/ClangQuery.cpp

123

return ASTStatus is clearly better!

clang-tools-extra/clang-query/tool/ClangQuery.cpp

int main(int argc, const char **argv) {
   if (!Commands.empty() && !CommandFiles.empty()) {
     llvm::errs() << argv[0] << ": cannot specify both -c and -f\n";
     return 1;
   }
 
   ClangTool Tool(OptionsParser.getCompilations(),
                  OptionsParser.getSourcePathList());
   std::vector<std::unique_ptr<ASTUnit>> ASTs;
-  if (Tool.buildASTs(ASTs) != 0)
+  int Status = Tool.buildASTs(ASTs);
+  if (Status == 1) {
+
+    // Building ASTs failed.
     return 1;
+  } else if (Status == 2) {
+    llvm::errs() << "Some files skipped, continuing" << "\n";
+  } else {
+    assert(Status == 0 && "Unexpected status returned: " && Status);
+  }
 
   QuerySession QS(ASTs);
 
   if (!Commands.empty()) {
     for (auto I = Commands.begin(), E = Commands.end(); I != E; ++I) {
       QueryRef Q = QueryParser::parse(*I, QS);
       if (!Q->run(llvm::outs(), QS))
         return 1;
while (llvm::Optional<std::string> Line = LE.readLine()) {
       QueryRef Q = QueryParser::parse(*Line, QS);
       Q->run(llvm::outs(), QS);
       llvm::outs().flush();
       if (QS.Terminate)
         break;
     }
   }
 
   return 0;
 }