Hiralo added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:337
+
+    Config.assign((*Text)->getBuffer());
+  }
----------------
DmitryPolukhin wrote:
> DmitryPolukhin wrote:
> > I suggest creating new local variable with text of the config from `Config` 
> > or `ConfigFile` file content i.e. avoid modifying `Config` itself.
> It doesn't compile and using local variable instead of changing command line 
> option will make this code easier to read and understand:
> ```
> clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:334:12: error: no 
> matching member function for call to 'assign'
>     Config.assign((*Text)->getBuffer());
>     ~~~~~~~^~~~~~
> ```
> Could you please fix build and please upload diff with `arc diff` as 
> @njames93 suggested so buildbot can test your changes?
> I suggest creating new local variable with text of the config from `Config` 
> or `ConfigFile` file content i.e. avoid modifying `Config` itself.

Sorry! probably I missed this comment!

std::string Temp = (*Text)->getBuffer();
Config.assign(Temp); // ???

if so how code will enter into below block... because we want to reuse 
code-path of 'Config' correct?

if (!Config.empty()) {
 if (llvm::ErrorOr<ClangTidyOptions> ParsedConfig = parseConfiguration(Config)) 
{
...
}

?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89936/new/

https://reviews.llvm.org/D89936

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

Reply via email to