smeenai added a comment.

In D64538#1579610 <https://reviews.llvm.org/D64538#1579610>, @rnk wrote:

> In D64538#1579573 <https://reviews.llvm.org/D64538#1579573>, @smeenai wrote:
>
> > In D64538#1579561 <https://reviews.llvm.org/D64538#1579561>, @rnk wrote:
> >
> > > An extremely convenient feature of the current escaping pattern is that 
> > > it magically works for both the cmd shell and various bash 
> > > implementations. You can simply copy paste the commands and run them. 
> > > This matters, for example, for the .sh crash reproducer script. On 
> > > Windows today, you can just run it with bash, and it works. So, I'd 
> > > prefer it if we didn't do this.
> > >
> > > If you want -### output to be more FileCheck friendly, then I would 
> > > recommend adding a new driver flag for testing that dumps the commands 
> > > without doing any escaping. This came up before:
> > >  https://reviews.llvm.org/D62493
> >
> >
> > As long as the backslashes aren't followed by a few special characters (as 
> > detailed in my comment), the backslash should still continue to work. I've 
> > been testing this with git bash and I'm still able to copy-paste the ouptut 
> > command.
>
>
> For the use case of crash reproduction, we often get these .sh files from 
> users, and then try to reproduce and reduce the crash on Linux. Right now 
> it's just a matter of adjusting the clang binary being run to reproduce a 
> crash, but with this change, it will be harder. The current escaping is kind 
> of the universal currency, a quoting style that is accepted everywhere.


I'm not fully understanding this. If the .sh file was generated on Windows, 
it'll contain backslashes in any arguments that are paths, right? Before this 
change, those backslashes would have been doubled up, but the backslash still 
wouldn't work as a path separator on Linux, so those arguments would need 
manual adjustment. I guess the other case that matters is if you have a 
backslash in an argument which isn't a path, in which case the old behavior was 
definitely correct and the new one definitely isn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64538



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to