[llvm-branch-commits] [clang-tools-extra] [clangd] check for synthesized symbols when tracking include locations (PR #75128)

[Matheus Izvekov via llvm-branch-commits]

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