kbobyrev requested changes to this revision.
kbobyrev added a comment.
This revision now requires changes to proceed.

Hi, thanks for flagging the issue! Could you please give more information where 
exactly the tests fail (e.g. logs with assertions)? This way I could understand 
where exactly the failure happens.

I can understand what you're trying to do but the patch introduces the 
behaviour which is unintended as of right now. The convention is to use POSIX 
paths everywhere in the Marshalling code since the end result will be a URI 
anyway. The patch changes it to Windows slashes which is orthogonal to this 
direction.

You're right about `llvm::Expected<clangd::FuzzyFindRequest> 
Marshaller::fromProtobuf` needing to convert to the native path though since 
there aren't any URIs in the end. Also, I think we should convert roots to the 
POSIX instead of asserting it.

I'm really curious what failures beyond `FuzzyFindRequest` are on Windows and 
I'd be happy to fix those but unfortunately I don't have a Windows machine to 
run the tests myself :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89529

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

Reply via email to