[PATCH] D54379: Add Hurd toolchain support to Clang

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

Commented on that particular idiom, there's two instances where it's used, aside from variable naming issues (`pos` should be `Pos`) it's very non idiomatic as far as rest of LLVM code goes, don't pass string literals around as `const char*`, prefer `StringRef` instead, that would also save you from needing to resort to using `strlen` later (sorry for previous comment, I didn't mean `strcpy`).



================
Comment at: lib/Driver/Driver.cpp:400
+
+  S.replace(pos, strlen(Other), Replace);
+}
----------------
Please avoid that kind of code and avoid `strlen`, you should pass things as StringRef as that's the general way of doing things unless you have a good reason for doing so otherwise. This entire function/part that uses it should be rewritten, I especially don't like the temporary `std::string` on the stack. It may be worth considering SmallString which is a variation of SmallVector or just manipulating the StringRef. You very certainly don't need `strlen` however, StringRef provides the needed operators, same goes for using `std::string::find`, just use StringRef instead. 


Repository:
  rC Clang

https://reviews.llvm.org/D54379