[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kbobyrev updated this revision to Diff 420423. kbobyrev added a comment. Switch to stable file UniqueIDs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123031/new/ https://reviews.llvm.org/D123031 Files: clang-tools-extra/clangd/IncludeCleaner.c

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:73 + auto Filename = SM.getFileEntryForID(SM.getFileID(Range.getBegin())) + ->tryGetRealPathName(); + Includes->addMapping(Filename, isLiteralInclud

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

sammccall added a comment. Changing the strings here makes sense, but I'm confused by the use of tryGetRealPathName. Also, we ended up dropping the filename in favor of UniqueID in IncludeStructure, so that's an alternative here. Yes, I think we need a test here with a scope wide enough that it

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kbobyrev added a comment. In D123031#3426382 , @kadircet wrote: >> Right! I think I also need to do that in SymbolCollector.cpp. > > Now that you mentioned this too, it feels like the issue is not about being > consistent, as I think all of the places th

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kadircet added a comment. > Right! I think I also need to do that in SymbolCollector.cpp. Now that you mentioned this too, it feels like the issue is not about being consistent, as I think all of the places that we have today are actually making use of `FileEntry::getName` as keys to `Canonica

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kbobyrev added a comment. In D123031#3426303 , @kadircet wrote: > note that you seem to be using `auto PublicHeader = > CanonIncludes.mapHeader(Entry->getName());` (i.e. `getName`) in D120306 > and not `tryGetRealPathN

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kadircet added a comment. note that you seem to be using `auto PublicHeader = CanonIncludes.mapHeader(Entry->getName());` (i.e. `getName`) in D120306 and not `tryGetRealPathName`. it might be nice to have a test case here, at least one that only fails/runs on

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kbobyrev added a comment. This is required for D120306 which was landed but later reverted because of the failing Windows tests (different slash types for filenames requested during preamble parsing and in the main file). Repository: rG LLVM Github Monorepo

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kbobyrev updated this revision to Diff 420175. kbobyrev added a comment. Fix the behavior: get Real Path instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123031/new/ https://reviews.llvm.org/D123031 Files: clang-tools-extra/clangd/index/Ca

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

kbobyrev created this revision. kbobyrev added a reviewer: sammccall. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. kbobyrev requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-