bnbarham added a comment.

In D121733#3392968 <https://reviews.llvm.org/D121733#3392968>, @ppluzhnikov 
wrote:

>> There's also some others where I wouldn't expect them to be failing in this 
>> patch, eg. the ones from `/` -> `{{[/\\]}}`.
>
> These are failing because `remove_dots` (un-intuitively) also changes 
> "foo/bar\\baz" to "foo\\bar\\baz"
>
>> Are there tests that we can't just fix to expect either `/` or `\\`?
>
> That's what this change is doing (but there is more Winx64 cleanup pending; 
> sorry, I should have sent this as a draft, but I am still learning the ropes 
> here).

Ah, I didn't realise that `remove_dots` also changed slashes.

>> Why do we need to change the underlying behaviour here at all?
>
> Sorry, I didn't understand that comment.

I was referring to the other patch here. I was under the impression that you 
started this patch to fix the failures in D121658 
<https://reviews.llvm.org/D121658>, but perhaps that isn't the case and you 
just thought this was a generally nice cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121733

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

Reply via email to