[PATCH] D90866: Fix bugs in EOL marking in command line tokenizers

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Support/CommandLine.cpp:872
         NewArgv.push_back(Saver.save(StringRef(Token)).data());
+      // Mark the end of lines in response files
+      if (MarkEOLs && C == '\n')
----------------
MaskRay wrote:
> Nit: full stop.
Fixed here and above


================
Comment at: llvm/unittests/Support/CommandLineTest.cpp:250
+TEST(CommandLineTest, TokenizeAndMarkEOLs) {
+  const char Input[] = "clang -Xclang foo\n\n foo\"bar\"baz\n x.cpp\n";
+  const char *const Output[] = {"clang", "-Xclang", "foo",
----------------
MaskRay wrote:
> Could you add some comments what nullptr is used for?
> 
> As another interesting test, does `-Xclang\nfoo` work?
Comments added. As far as `-Xclang\nfoo` goes, this code produces the expected result: `"-Xclang", nullptr, "foo"`. I'll adjust the test a bit, but I think it already covers this case. As for what clang does with that, I expect it doesn't work, but I don't think it's important to make it work. -Xclang is clang's own option, and if it doesn't support being split across newlines, that's our choice, as long as we don't crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90866