mizvekov wrote: > Hi @mizvekov thanks for the fix, but I am not sure if this is at the right > level. The way you're bailing out currently prevents clangd from indexing all > implicit definitions, even if we have a hard-coded mapping for them (based on > the symbol name). > > Also the map you're preventing the insertion stores FileIDs as values, not > keys, hence it isn't the place firing assertion failures. Can you instead > move the logic to SymbolCollector::finish, which uses FileIDs to determine > includers for objective-c symbols? we should only perform that logic if > there's a valid FileID.
I see. Though the situation you describe seems impossible at the present state of the implementation. For standard library stuff, we have hardcoded std::move and std::remove, implemented [here](https://github.com/llvm/llvm-project/blob/6d8fe3dc9a4f6225c4c84de578469efc50d7684d/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp#L117) but those require some sort of user provided declaration somewhere in code, which will have a valid source location, and we indeed unit test that. As an aside, I think in the above case we should honor the `-freestanding` flag, but currently we do not. For the builtin-which-requires-include case, that is unimplemented, with a FIXME note: https://github.com/llvm/llvm-project/blob/6d8fe3dc9a4f6225c4c84de578469efc50d7684d/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp#L177 I agree in principle to the change you suggested, even if presently non testable, but I still think it makes sense to skip inserting invalid FileIDs into the `IncludeFiles` map. https://github.com/llvm/llvm-project/pull/75128 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
