[PATCH] D122914: [Windows] Fix handling of \" in program name on cmd line.

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added inline comments.


================
Comment at: llvm/lib/Support/CommandLine.cpp:1027-1028
 
   if (State == UNQUOTED)
     AddToken(Saver.save(Token.str()));
 }
----------------
hans wrote:
> simon_tatham wrote:
> > rnk wrote:
> > > This seems like it could be a bug when the command line ends with an open quoted string. We should match the CRT logic. In the original example, the entire command line would become argv[0].
> > I thought the same, and I considered fixing it as part of the same commit – I would have expected that we'd add the current token if it had accumulated any text, whether the quotes had been closed or not. But the git history tells me that D78346 changed it from that to this, so I didn't want to unilaterally revert a previous deliberate change.
> > 
> > However, now I look more closely at that previous change, it looks as if it's not intentionally discarding any actual text – it's _preventing_ the discarding of an explicit empty-string token. So perhaps I can fix that after all without reverting the intended change. I think we ought to be testing `State != INIT`.
> Perhaps this should be done in a separate commit?
OK, I split it up at the last minute, and committed rG1be024ee450f2d3cb07086f6141d50f291c1910b for the problem with a trailing unclosed quoted string, and rG32814df442690d4673759296d850804773a7ea5b for the problem with \" in the command name.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122914