[PATCH] D42361: [Tooling] Returns non-zero status code when files are skipped.

[Eric Liu via Phabricator via cfe-commits]

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