Page MenuHomePhabricator

Add the notion of deferred diagnostics.
ClosedPublic

Authored by jlebar on Aug 6 2016, 12:38 PM.

Details

Summary

This patch lets you create diagnostics that are emitted if and only if a
particular FunctionDecl is codegen'ed.

This is necessary for CUDA, where some constructs -- e.g. calls from
host+device functions to host functions when compiling for device -- are
allowed to appear in semantically-correct programs, but only if they're
never codegen'ed.

Diff Detail

Event Timeline

jlebar updated this revision to Diff 67082.Aug 6 2016, 12:38 PM
jlebar retitled this revision from to Add the notion of deferred diagnostics..
jlebar updated this object.
jlebar added a reviewer: rnk.
jlebar added subscribers: tra, cfe-commits.
tra added inline comments.Aug 8 2016, 11:25 AM
clang/lib/CodeGen/CodeGenModule.cpp
2886

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.

jlebar updated this revision to Diff 67367.Aug 9 2016, 10:22 AM
jlebar marked an inline comment as done.

Address review comments, and don't abort codegen'ing a function if it has only
deferred warnings -- check specifically for errors.

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.

clang/lib/CodeGen/CodeGenModule.cpp
2886

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.)

rnk edited edge metadata.Aug 12 2016, 6:46 PM

Sorry for the delay, was trying to stay focused on debugging

clang/include/clang/AST/Decl.h
1647

Decl memory is pretty precious, I think you'll be better off with a map from Decl* to TinyPtrVector in the ASTContext. Just pass the ASTContext into takeDeferredDiags.

jlebar updated this revision to Diff 68063.Aug 15 2016, 1:07 PM
jlebar edited edge metadata.

Move deferred diags storage into ASTContext.

rnk accepted this revision.Aug 15 2016, 1:20 PM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Aug 15 2016, 1:20 PM
This revision was automatically updated to reflect the committed changes.