ioeric added inline comments.

================
Comment at: include/clang/Tooling/Tooling.h:331
+  /// \returns 0 on success; 1 if any error occured; 2 if there is no error but
+  /// some files are skipped due to missing compile commands.
   int run(ToolAction *Action);
----------------
hokein wrote:
> I'm not sure whether this new behavior will cause any regression issues (many 
> tools invoke this method like `if (Tool.run(...))`)
> 
> * before the patch, return 0 if some files are skipped (we think it succeeds)
> * after the patch, return 2 if some files are skipped (we think it fails)
> 
> Maybe @bkramer has more words on it?
I should have mentioned in the path summary that I think this would not regress 
tools. I checked call sites of this API. I think it's reasonable for most tools 
to treat skipping files as failure (if they care about failure; some tools 
simply ignore the status code). Tools that care about this are already ignoring 
the failures (e.g. 
https://github.com/llvm-mirror/clang/blob/master/tools/clang-refactor/ClangRefactor.cpp#L630).
 But there might be something that I am missing. I would certainly like more 
opinions on this.


Repository:
  rC Clang

https://reviews.llvm.org/D42361



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to