[PATCH] D29940: Allow use of spaces in Bugpoint ‘--compile-command’ argument
Matthias Braun via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 16 13:04:21 PST 2017
MatzeB added a comment.
Looks generally good, but I'd like to hear a comment on the string handling:
================
Comment at: ToolRunner.cpp:379-381
+ char Delimiter = ' ';
+ char Escape = '\\';
+ char End = '\0';
----------------
Note sure we gain much by moving those 3 characters into a variables (though admittedly already better than the previous code that was searching around in a 1-character std::string).
================
Comment at: ToolRunner.cpp:391
+ if (Escape == CommandLine[Pos]) {
+ if (End != CommandLine[Pos + 1]) {
+ Token.push_back(CommandLine[++Pos]);
----------------
MatzeB wrote:
> We omit `{}` for `if`/`for` where possible.
For my understanding: CommandLine is an std::string with the last character being '\0'? (It's a bit confusing as I would either expect an std::string with a len but without terminating character or a `const char *` with terminating character...)
================
Comment at: ToolRunner.cpp:391-393
+ if (End != CommandLine[Pos + 1]) {
+ Token.push_back(CommandLine[++Pos]);
+ }
----------------
We omit `{}` for `if`/`for` where possible.
================
Comment at: ToolRunner.cpp:397-399
+ if (Token.empty()) {
+ continue;
+ }
----------------
No need to braces.
Repository:
rL LLVM
https://reviews.llvm.org/D29940
More information about the llvm-commits
mailing list