Page MenuHomePhabricator

[driver] add a new option `-gen-cdb-fragment-path` to emit a fragment of a compilation database for each compilation
ClosedPublic

Authored by arphaman on Wed, Aug 21, 1:46 PM.

Details

Summary

This patch adds a new option called -gen-cdb-fragment-path to the driver, which can be used to specify a directory path to which clang can emit a fragment of a CDB for each compilation it needs to invoke.

The CDB fragment is emitted into a unique file in the specified directory. It contains the -cc1 clang invocation in its command. The file itself is actually a valid standalone CDB (if you disregard the not yet well supported -cc1 innovation by the CDB, which I'll fix). To load the full CDB that can be emitted during a build, I'm going to add a new CDB reader from a directory that reads all fragments and aggregates them into one CDB in a follow-up patch.

This is useful to setup verification infrastructure for clang-scan-deps for some projects that don't have a way of constructing a CDB from their build.

Diff Detail

Event Timeline

arphaman created this revision.Wed, Aug 21, 1:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Aug 21, 1:46 PM
jkorous added inline comments.Wed, Aug 21, 2:20 PM
clang/lib/Driver/Driver.cpp
1044 ↗(On Diff #216466)

IIUC this feature might end up being used from some test script. I'm wondering - what would happen had the argument been an empty string?

clang -c foo.c -gen-cdb-fragment-path ""
4868 ↗(On Diff #216466)

I am just wondering whether we shouldn't notice user about failures, possibly with some error message.

4896 ↗(On Diff #216466)

It feels like this might have been already implemented somewhere else.

Couldn't we use for example llvm::yaml::escape()?

https://llvm.org/doxygen/namespacellvm_1_1yaml.html#aefed9cb3f107aee0cff4d325c7d689ae

How does it relate to the -MJ option?

How does it relate to the -MJ option?

Ah, I wasn't aware that such option existed. I should use it instead.

arphaman updated this revision to Diff 216717.Thu, Aug 22, 2:17 PM

Reimplement the new option on top of -MJ. -MJ is still the preferred option if both are specified.

@jkorous I addressed your comments as well ("" directory is now allowed, error is reported, existing escape used).

jkorous added inline comments.Thu, Aug 22, 6:43 PM
clang/lib/Driver/ToolChains/Clang.cpp
2068

Do we still need this now?

2090

Just to make sure - the error different on purpose here?

2093

I'm not familiar with intended lifecycle of CompilationDatabase member. Just to make sure - we need neither to preserve the original value nor reset the stream after we're done here (as in - nobody is going to write anything after we're done here), right?

jkorous added inline comments.Thu, Aug 22, 6:46 PM
clang/lib/Driver/ToolChains/Clang.h
99

BTW this looks kind of funny const InputInfo &Output, const InputInfo &Input.

arphaman updated this revision to Diff 216898.Fri, Aug 23, 10:40 AM

Address review comments.

arphaman marked 5 inline comments as done.Fri, Aug 23, 10:42 AM
arphaman added inline comments.
clang/lib/Driver/ToolChains/Clang.cpp
2068

No, the CWD should actually be read in DumpCompilationDatabase. I updated that method.

2090

Not really needed I guess, I reused the error.

2093

Yes, we should either preserve or not reset the stream once it's created. Multiple clang invocations can be constructed from one driver run, so we want to keep the stream open while the driver is running.

clang/lib/Driver/ToolChains/Clang.h
99

Yes, that's unfortunate type naming in Clang :(

jkorous accepted this revision.Fri, Aug 23, 6:09 PM
This revision is now accepted and ready to land.Fri, Aug 23, 6:09 PM
This revision was automatically updated to reflect the committed changes.
arphaman marked an inline comment as done.