[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