[PATCH] D51260: Extract runCommandsInFile method

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 27 04:33:23 PDT 2018


aaron.ballman added a comment.

I'm not opposed to the changes here, but I am wondering what the benefits are to splitting this off into its own function?



================
Comment at: clang-query/tool/ClangQuery.cpp:61
 
+int runCommandsInFile(const char* exeName, std::string const& fileName, QuerySession& QS)
+{
----------------
aaron.ballman wrote:
> You should run your patch through clang-format to properly format only the parts that you've changed. Also, it should be `ExeName` and `FileName` per the coding standard.
I think this function should return a `bool` instead of an `int`. The caller can decide how to translate failure into a return code.


================
Comment at: clang-query/tool/ClangQuery.cpp:61-62
 
+int runCommandsInFile(const char* exeName, std::string const& fileName, QuerySession& QS)
+{
+    std::ifstream Input(fileName.c_str());
----------------
You should run your patch through clang-format to properly format only the parts that you've changed. Also, it should be `ExeName` and `FileName` per the coding standard.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51260





More information about the cfe-commits mailing list