Skip to content

Conversation

AtomicOperation
Copy link
Contributor

A PR for compilation database changes were reverted because of this issue upstream. Re-applying with the fix, which is to pass in the original target set as the target_universe for the rdeps call.

Original PR summary:

We're starting to generate and use compile_commands.json, and one of the problems we have is that the compilation command for a header file is based on the target that exports it. Most of the time this is fine, but there are cases where we resolve circular dependencies by moving some dependencies out of the header target, which means they're missing from the compile commands.

Example:

If we have two libraries, Foo and Bar. Initially, Foo.cpp includes Bar.h and Bar.cpp includes Foo.h. To solve that, we create Foo:public-headers and Bar:public-headers and depend on those.

But if Foo.h (not cpp) includes Bar1.h and Bar2.h includes Foo.h, then having public headers isn't enough. In that case, we remove the dependencies from the public header targets (since they will produce circular dependency errors) and manually add them to the private dependencies of any consumers.

The problem we have right now is that in this case, the compile commands for Foo.h will be generated based on the Foo:public-headers target, which doesn't have Bar in its public dependencies. As a result, the IDE shows errors because essentially the file doesn't compile on its own, even if in practice it always compiles in every case where we use it.

The fix we've implemented is to check targets to see if they have actual source files, and if not, choose an rdep in the same file that does.

Especially since this is a heuristic, it may not be a good fit for upstreaming. If nothing else this can be something that other users can look at and pull from if they have a similar problem.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 15, 2025
Copy link
Contributor

meta-codesync bot commented Oct 15, 2025

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D84750421. (Because this pull request was imported automatically, there will not be any future comments.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant