jansvoboda11 added inline comments.

================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:684-685
+  if (!GeneratedStringsEqual)
+    llvm::report_fatal_error("Mismatch between arguments generated during "
+                             "round-trip");
+
----------------
dexonsmith wrote:
> jansvoboda11 wrote:
> > dexonsmith wrote:
> > > Another option vs. report_fatal_error would be to create (fatal) clang 
> > > error diagnostics. That applies here, and to my previous comments. WDYT? 
> > > (@Bigcheese, any thoughts?)
> > > 
> > > Also, I think `-round-trip-args-debug` might not be super discoverable. 
> > > Maybe we can add a comment in the code where the errors are reported 
> > > suggesting adding that.
> > If we decide to go with custom diagnostics instead of just using 
> > `llvm::errs` and `llvm::report_fatal_error`, we'd need to instantiate a 
> > custom `DiagnosticsEngine` here in `RoundTrip`, because we cannot rely on 
> > clients passing us reasonable `Diags`.
> > 
> > One such example is `CompilerInvocationTest`, where we don't care what 
> > diagnostics get emitted (by using `TextDiagnosticBuffer` consumer). The 
> > errors we're emitting here would be still useful to see when testing though.
> I'm not entirely following why this would need a custom Diags. If the client 
> wants to ignore diagnostics that should be up to the client, especially in 
> clang-as-a-library situations. In fact, using `errs` or `dbgs` at all is 
> pretty iffy when clang is used as a library.
> 
> For CompilerInvocationTest, I imagine we could change the consumer / test 
> fixture / etc. somehow to capture this case.
My understanding is that we'd be converting `llvm::report_fatal_error` into 
error diagnostics and `PrintArgs` into notes and remarks.

The hiccup here is that `Diags` most probably hasn't been configured yet (we're 
still just parsing the `DiagnosticsOptions`). And by default, 
`DiagnosticsEngine` ignores all remarks.

So my thinking was that to ensure our remarks are not thrown away, we could 
modify the existing `Diags` to our liking (too invasive) or set up a custom 
`DiagnosticsEngine` instance here in `RoundTrip`. In the end, the best solution 
is probably to do this of setup in `cc1_main`, so that we're emitting 
diagnostics only when used within compiler.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94472/new/

https://reviews.llvm.org/D94472

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to