[PATCH] D23241: Add the notion of deferred diagnostics.

Justin Lebar via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 9 10:23:20 PDT 2016


jlebar added a comment.

Reid, I'd still like you to have a look at this one if you don't mind, since it's outside my and Art's core competencies.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2886
@@ +2885,3 @@
+  // Check if this function has diagnostics that should be emitted when we
+  // codegen it.  If so, don't eit this function definition, but don't emit the
+  // diags just yet.  Emitting an error during codegen stops codegen, and we
----------------
tra wrote:
> eit->emit.  
> "don't do X, but don't do Y" construction sounds awkward to me.
> I'd reword the whole comment in terms of what the code does -- if there are diagnostics, only collect them to emit at the end of codegen. Otherwise, proceed to emit function definition.
> 
Thanks.  I tried to reword it, phal.

I also realized that this was skipping functions if they contained *any* deferred diags, but that's not right -- we only want to skip functions that contain deferred errors.  Fixed that too.

I wish I could use StoredDiagnostic instead of PartialDiagnosticAt, but I don't see a way to create a StoredDiagnostic for an error without setting the HasError bit in the DiagnosticEngine.  I need to carefully avoid setting that bit, otherwise we don't even make it to codegen.  (In fact it looks like the code for emitting diagnostics assumes that creating a StoredDiagnostic sets the bit, because I don't see it setting that bit when we emit the StoredDiagnostic.)


https://reviews.llvm.org/D23241





More information about the cfe-commits mailing list