[clang-tools-extra] [clangd] Add includes from source to non-self-contained headers (PR #72479)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 20 01:25:34 PST 2023


kadircet wrote:

hi! thanks for the interest @sr-tream but I am afraid this is likely to cause disruption in more cases than it might improve.

apart from technical details like the threading concerns and reliance on certain variants that we don't really have (eg order of includes); at a high level the idea of "finding a representative source file for the header and replicating the PP state" is hard to work in general.

The current approach of finding a candidate through `HeaderIncluders` will pick an arbitrary source file that transitively includes the header you're interested in. Hence you'll actually create a set of `-include XX` commands, that **might** (and probably will) recursively include the header itself. Also there's nothing detecting a failure in processing of the compilation unit, this is done without checking self-containedness (which is again hard to check) hence will definitely regress both performance and correctness for self-contained headers as well.

sorry for forgetting to hit send on friday, i see that you did some more iterations on the weekend. but i think it'd be better to discuss the general approach first, to see if that makes sense. not only to ensure non-self-contained headers can be handled properly rather than adding an edge case where things look like they're working (but they're not, hence resulting in more bug reports and hard-to-maintain fixes), but also to make sure they're not regressing anything for regular code paths.

https://github.com/llvm/llvm-project/pull/72479


More information about the cfe-commits mailing list