[PATCH] Driver: Include driver diags when we --serialize-diagnostics

Justin Bogner mail at justinbogner.com
Thu Oct 23 15:30:49 PDT 2014


Argyrios Kyrtzidis <kyrtzidis at apple.com> writes:
> +std::unique_ptr<DiagnosticsEngine> SDiagsWriter::createMetaDiags() {
> +  // FIXME: It's slightly absurd to create a new diagnostics engine here, but
> +  // the other options that are available today are worse:
> +  //
> +  // 1. Teach DiagnosticsConsumers to emit diagnostics to the engine they are a
> +  //    part of. The DiagnosticsEngine would need to know not to send
> +  //    diagnostics back to the consumer that failed. This would require us to
> +  //    rework ChainedDiagnosticsConsumer and teach the engine about multiple
> +  //    consumers, which is difficult today because most APIs interface with
> +  //    consumers rather than the engine itself.
> +  //
> +  // 2. Pass a DiagnosticsEngine to SDiagsWriter on creation - this would need
> +  //    to be distinct from the engine the writer was being added to and would
> +  //    normally not be used.
> +  IntrusiveRefCntPtr<DiagnosticIDs> IDs(new DiagnosticIDs());
> +  auto Client = new TextDiagnosticPrinter(llvm::errs(), State->DiagOpts.get());
> +  auto MetaDiags =
> +      llvm::make_unique<DiagnosticsEngine>(IDs, State->DiagOpts.get(), Client);
> +  return MetaDiags;
> +}
>
> Do we need to create this every time ? Why not have
> SDiagsWriter::getMetaDiags() that creates it on demand and reuses it ?

Sure, I've added the pointer to the writer State and renamed this to
getMetaDiags.

> diff --git a/test/Misc/serialized-diags-driver.c b/test/Misc/serialized-diags-driver.c
> new file mode 100644
> index 0000000..1109eb8
> --- /dev/null
> +++ b/test/Misc/serialized-diags-driver.c
> @@ -0,0 +1,21 @@
> +// Test that the driver correctly combines its own diagnostics with CC1's in the
> +// serialized diagnostics. To test this, we need to trigger diagnostics from
> +// both processes, so we compile code that has a warning (with an associated
> +// note) and then force the driver to crash. We compile stdin so that the crash
> +// doesn't litter the user's system with preprocessed output.
>
> How about using a test case that can occur normally, where you pass "-unknown-argument” to the driver and test that "warning: argument unused during compilation” shows up in the serialized diagnostics along with the diagnostics from the source file ?

That's a *much* better idea. Thank you. Changed the test to call clang
with -Wx-unknown-warning instead of forcing a crash.

> Otherwise, LGTM!

Committed in r220525. Thanks!

>> On Oct 20, 2014, at 10:43 AM, Justin Bogner <mail at justinbogner.com> wrote:
>> 
>> Currently, when --serialize-diagnostics is passed this only includes the
>> diagnostics from clang -cc1, and driver diagnostics are dropped. This
>> causes issues for tools that use the serialized diagnostics, since
>> stderr is lost and these diagnostics aren't seen at all.
>> 
>> This patch handles this by merging the diagnostics from the CC1 process
>> and the driver diagnostics into a single file when the driver invokes
>> CC1.
>> 
>> <serialized-diags.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list