Page MenuHomePhabricator

[clangd] Include scanner that finds compile commands for headers.
Needs ReviewPublic

Authored by sammccall on Jan 10 2018, 11:04 AM.

Details

Reviewers
ilya-biryukov
Summary

Not enabled because we need a threadsafe way to change VFS working directories.

Event Timeline

sammccall created this revision.Jan 10 2018, 11:04 AM
nridge added a subscriber: nridge.May 8 2019, 7:45 PM

Are there are plans to move forward with this sort of an include scanning approach?

Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2019, 7:45 PM
klimek added inline comments.May 8 2019, 10:40 PM
clangd/IncludeScanner.cpp
75

Usually I'd try to not lock a loop, as a large number of compile commands now blocks other threads for a bit. Fairness might not matter here, though.

97

I'd find it a bit more idiomatic to have two unique_locked blocks instead (easier to pull out a function when things grow etc), but I see it's annoying to have to default-construct an entry in that case :(
Given that I'd probably pull that into a function, but then we need to communicate Done. Sigh.

100

I'd try to write the code so multiple consumer threads would work - given that process() can't fail, why not pop in the locked session above?

131

I assume that's a note to you that you want to change this?

159

Wondering why we can't use one of the tooling abstractions here...

sammccall marked 2 inline comments as done.May 9 2019, 12:28 AM

Context here, this patch is over a year old and predates the work on heuristically picking a compile command from another file in the CDB.

I don't think anyone has concrete plans to revive this, the heuristics work well enough and often enough. I should probably have abandoned this patch, sorry for the time lost in detailed comments.

However there's certainly some value in indexing the include graph and/or determining main-file/header pairs.
e.g. to improve switchsourceheader, to enable cross-file refactorings like "out-line implementation". And maybe to improve compile command accuracy too.

This patch was pretty narrowly tailored at getting compile commands quickly, if we developed this approach it should probably be more general purpose, part of the index, and maybe take symbol declaration/definition into account rather than just #includes.

clangd/IncludeScanner.cpp
131

This has since been fixed, it's no longer global state :-)

159

As far as I can tell, the only abstraction Tooling offers here is CompileCommand, the only way to robustly manipulate it is through the Driver (the semantics here are too subtle for brute-force string manipulation). InterpolatingCompilationDatabase does this, but doesn't expose the primitives publicly. It could if there was a need.