mstorsjo added a comment.

Sorry for taking so long to get back to you on this matter...

In D113268#3112026 <https://reviews.llvm.org/D113268#3112026>, @sammccall wrote:

>> For someone unfamiliar with the tool, is there a simple "smoke test 
>> procedure" I could try out with it to kick the tyres?
>
> There's `clangd --check=some_file.cc` which simulates opening up a file in an 
> editor and clicking around in it.
> It does require a `compile_command.json` in a parent directory to know about 
> build flags.
> To try it out for real you need to connect an editor to it with a plugin 
> (like vscode-clangd).

Thanks! I tested both `clangd --check=some_file.cc` and vscode-clangd with a 
build of clangd set to prefer forward slashes, and it all worked fine so far. I 
didn't do any exhaustive use of it in vscode, but at least regular use seemed 
just fine.

> But realistic problems with paths tend to come from interactions with 
> external sources.
> For example, we had a bug that involved:
>
> - vscode sending paths like `file://c:/test.cc` (lowercase C), which made its 
> way into our AST cache
> - cmake creating `compile_commands.json` like `C:\test.cc` (uppercase C), 
> which made its way into our index
> - when renaming a symbol, results from the two weren't deduplicated against 
> each other so the edit was applied twice

Yup - I've seen lots of similar issues in other clang tests too. The best way 
forward to fix those would probably be to add some helper to `llvm::sys::path` 
for doing path comparisons, which would ignore case differences (on windows and 
macos) and/or separator style differences (on windows). But

Anyway, this change (and a build that prefers generating paths with forward 
slashes) seems to work as expected with regards to that - and I updated the 
patch with what was requested.

The whole setup for this mode isn't settled, so there's no buildbot for this 
mode (yet), we'll see if we end up setting one up. In any case, most of this 
change shouldn't be too intrusive I hope.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113268

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

Reply via email to