This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Use flags from open files when opening headers they include
ClosedPublic

Authored by sammccall on Feb 23 2021, 6:26 PM.

Details

Summary

Currently our strategy for getting header compile flags is something like:

A) look for flags for the header in compile_commands.json

This basically never works, build systems don't generate this info.

B) try to match to an impl file in compile_commands.json and use its flags

This only (mostly) works if the headers are in the same project.

C) give up and use fallback flags

This kind of works for stdlib in the default configuration, and
otherwise doesn't.

Obviously there are big gaps here.

This patch inserts a new attempt between A and B: if the header is
transitively included by any open file (whether same project or not),
then we use its compile command.

This doesn't make any attempt to solve some related problems:

  • parsing non-self-contained header files in context (importing PP state)
  • using the compile flags of non-opened candidate files found in the index

Fixes https://github.com/clangd/clangd/issues/123
Fixes https://github.com/clangd/clangd/issues/695
See https://github.com/clangd/clangd/issues/519

Diff Detail

Event Timeline

sammccall created this revision.Feb 23 2021, 6:26 PM
sammccall requested review of this revision.Feb 23 2021, 6:26 PM
sammccall updated this revision to Diff 326042.Feb 24 2021, 3:43 AM

Record memory usage

nridge added a subscriber: nridge.Feb 27 2021, 11:55 PM
hokein accepted this revision.Feb 28 2021, 1:48 PM

thanks, this looks great.

clang-tools-extra/clangd/TUScheduler.cpp
187

I'm a bit confused, I don't get the meaning of the "This could also naturally live in the index", what lives in the index? my best guess is the include structure captured in background index.

202

maybe it is just me, it took me a while to understand the meaning of "invalidates the preamble" -- switching proxies usually indicates the change of CMD of the header, thus the preamble of the header is invalidated, is that right?

This revision is now accepted and ready to land.Feb 28 2021, 1:48 PM
This revision was landed with ongoing or failed builds.Mar 1 2021, 12:44 AM
This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.