[PATCH] D23241: Add the notion of deferred diagnostics.
jlebar created this revision. jlebar added a reviewer: rnk. jlebar added subscribers: tra, cfe-commits. 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. https://reviews.llvm.org/D23241 Files: clang/include/clang/AST/Decl.h clang/lib/AST/Decl.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h Index: clang/lib/CodeGen/CodeGenModule.h === --- clang/lib/CodeGen/CodeGenModule.h +++ clang/lib/CodeGen/CodeGenModule.h @@ -490,6 +490,10 @@ /// MDNodes. llvm::DenseMap MetadataIdMap; + /// Diags gathered from FunctionDecl::takeDeferredDiags(). Emitted at the + /// very end of codegen. + std::vector> DeferredDiags; + public: CodeGenModule(ASTContext &C, const HeaderSearchOptions &headersearchopts, const PreprocessorOptions &ppopts, Index: clang/lib/CodeGen/CodeGenModule.cpp === --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -497,6 +497,16 @@ EmitVersionIdentMetadata(); EmitTargetMetadata(); + + // Emit any deferred diagnostics gathered during codegen. We didn't emit them + // when we first discovered them because that would have halted codegen, + // preventing us from gathering other deferred diags. + for (const PartialDiagnosticAt &DiagAt : DeferredDiags) { +SourceLocation Loc = DiagAt.first; +const PartialDiagnostic &PD = DiagAt.second; +DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID())); +PD.Emit(Builder); + } } void CodeGenModule::UpdateCompletedType(const TagDecl *TD) { @@ -2872,6 +2882,19 @@ llvm::GlobalValue *GV) { const auto *D = cast(GD.getDecl()); + // 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 + // want to display as many deferred diags as possible. We'll emit the now + // twice-deferred diags at the very end of codegen. + auto Diags = D->takeDeferredDiags(); + bool HasDiags = !Diags.empty(); + DeferredDiags.insert(DeferredDiags.end(), + std::make_move_iterator(Diags.begin()), + std::make_move_iterator(Diags.end())); + if (HasDiags) +return; + // Compute the function info and LLVM type. const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD); llvm::FunctionType *Ty = getTypes().GetFunctionType(FI); Index: clang/lib/AST/Decl.cpp === --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -3436,6 +3436,22 @@ return 0; } +void FunctionDecl::addDeferredDiag(PartialDiagnosticAt PD) { + if (!DeferredDiags) +DeferredDiags = llvm::make_unique>(); + DeferredDiags->emplace_back(PD); +} + +std::vector FunctionDecl::takeDeferredDiags() const { + if (!DeferredDiags) +return {}; + assert(!DeferredDiags->empty() && + "DeferredDiags should be non-null only if it's also non-empty."); + auto Ret = std::move(*DeferredDiags); + DeferredDiags.reset(); + return Ret; +} + //===--===// // FieldDecl Implementation //===--===// Index: clang/include/clang/AST/Decl.h === --- clang/include/clang/AST/Decl.h +++ clang/include/clang/AST/Decl.h @@ -1638,6 +1638,14 @@ /// declaration name embedded in the DeclaratorDecl base class. DeclarationNameLoc DNLoc; + /// Storage for diagnostics deferred until this function is codegen'ed (if it + /// ever is). + /// + /// These are rarely used, so we use a pointer-to-vector to save two words + /// inside FunctionDecl. This is mutable because emitting diagnostics (which + /// clears this list) needs to be a logically-const operation. + mutable std::unique_ptr> DeferredDiags; + /// \brief Specify that this function declaration is actually a function /// template specialization. /// @@ -2271,6 +2279,14 @@ /// returns 0. unsigned getMemoryFunctionKind() const; + /// Add a diagnostic to be emitted if and when this function is codegen'ed. + void addDeferredDiag(PartialDiagnosticAt PD); + + /// Gets this object's list of deferred diagnostics, if there are any. + /// + /// Although this is logically const, it clears our list of deferred diags. + std::vector takeDeferredDiags() const; + // Implement
Re: [PATCH] D23241: Add the notion of deferred diagnostics.
tra added inline comments. 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 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. https://reviews.llvm.org/D23241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23241: Add the notion of deferred diagnostics.
jlebar updated this revision to Diff 67367. jlebar marked an inline comment as done. jlebar added a comment. Address review comments, and don't abort codegen'ing a function if it has only deferred warnings -- check specifically for errors. https://reviews.llvm.org/D23241 Files: clang/include/clang/AST/Decl.h clang/lib/AST/Decl.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h Index: clang/lib/CodeGen/CodeGenModule.h === --- clang/lib/CodeGen/CodeGenModule.h +++ clang/lib/CodeGen/CodeGenModule.h @@ -490,6 +490,10 @@ /// MDNodes. llvm::DenseMap MetadataIdMap; + /// Diags gathered from FunctionDecl::takeDeferredDiags(). Emitted at the + /// very end of codegen. + std::vector> DeferredDiags; + public: CodeGenModule(ASTContext &C, const HeaderSearchOptions &headersearchopts, const PreprocessorOptions &ppopts, Index: clang/lib/CodeGen/CodeGenModule.cpp === --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -497,6 +497,16 @@ EmitVersionIdentMetadata(); EmitTargetMetadata(); + + // Emit any deferred diagnostics gathered during codegen. We didn't emit them + // when we first discovered them because that would have halted codegen, + // preventing us from gathering other deferred diags. + for (const PartialDiagnosticAt &DiagAt : DeferredDiags) { +SourceLocation Loc = DiagAt.first; +const PartialDiagnostic &PD = DiagAt.second; +DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID())); +PD.Emit(Builder); + } } void CodeGenModule::UpdateCompletedType(const TagDecl *TD) { @@ -2872,6 +2882,33 @@ llvm::GlobalValue *GV) { const auto *D = cast(GD.getDecl()); + // Emit this function's deferred diagnostics, if none of them are errors. If + // any of them are errors, don't codegen the function, but also don't emit any + // of the diagnostics just yet. Emitting an error during codegen stops + // further codegen, and we want to display as many deferred diags as possible. + // We'll emit the now twice-deferred diags at the very end of codegen. + // + // (If a function has both error and non-error diags, we don't emit the + // non-error diags here, because order can be significant, e.g. with notes + // that follow errors.) + auto Diags = D->takeDeferredDiags(); + bool HasError = llvm::any_of(Diags, [this](const PartialDiagnosticAt &PDAt) { +return getDiags().getDiagnosticLevel(PDAt.second.getDiagID(), PDAt.first) >= + DiagnosticsEngine::Error; + }); + if (HasError) { +DeferredDiags.insert(DeferredDiags.end(), + std::make_move_iterator(Diags.begin()), + std::make_move_iterator(Diags.end())); +return; + } + for (PartialDiagnosticAt &PDAt : Diags) { +const SourceLocation &Loc = PDAt.first; +const PartialDiagnostic &PD = PDAt.second; +DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID())); +PD.Emit(Builder); + } + // Compute the function info and LLVM type. const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD); llvm::FunctionType *Ty = getTypes().GetFunctionType(FI); Index: clang/lib/AST/Decl.cpp === --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -3436,6 +3436,22 @@ return 0; } +void FunctionDecl::addDeferredDiag(PartialDiagnosticAt PD) { + if (!DeferredDiags) +DeferredDiags = llvm::make_unique>(); + DeferredDiags->emplace_back(PD); +} + +std::vector FunctionDecl::takeDeferredDiags() const { + if (!DeferredDiags) +return {}; + assert(!DeferredDiags->empty() && + "DeferredDiags should be non-null only if it's also non-empty."); + auto Ret = std::move(*DeferredDiags); + DeferredDiags.reset(); + return Ret; +} + //===--===// // FieldDecl Implementation //===--===// Index: clang/include/clang/AST/Decl.h === --- clang/include/clang/AST/Decl.h +++ clang/include/clang/AST/Decl.h @@ -1638,6 +1638,14 @@ /// declaration name embedded in the DeclaratorDecl base class. DeclarationNameLoc DNLoc; + /// Storage for diagnostics deferred until this function is codegen'ed (if it + /// ever is). + /// + /// These are rarely used, so we use a pointer-to-vector to save two words + /// inside FunctionDecl. This is mutable because emitting diagnostics (which + /// clears this list) needs to be a logically-const operation. + mutable std::unique_ptr> DeferredDiags; + /// \brief Specify that this function declaration is actually a function /// template specialization. /// @@ -2271
Re: [PATCH] D23241: Add the notion of deferred diagnostics.
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23241: Add the notion of deferred diagnostics.
rnk added a comment. Sorry for the delay, was trying to stay focused on debugging Comment at: clang/include/clang/AST/Decl.h:1647 @@ +1646,3 @@ + /// clears this list) needs to be a logically-const operation. + mutable std::unique_ptr> DeferredDiags; + 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. https://reviews.llvm.org/D23241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23241: Add the notion of deferred diagnostics.
jlebar updated this revision to Diff 68063. jlebar added a comment. Move deferred diags storage into ASTContext. https://reviews.llvm.org/D23241 Files: clang/include/clang/AST/ASTContext.h clang/include/clang/AST/Decl.h clang/lib/AST/Decl.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h Index: clang/lib/CodeGen/CodeGenModule.h === --- clang/lib/CodeGen/CodeGenModule.h +++ clang/lib/CodeGen/CodeGenModule.h @@ -490,6 +490,10 @@ /// MDNodes. llvm::DenseMap MetadataIdMap; + /// Diags gathered from FunctionDecl::takeDeferredDiags(). Emitted at the + /// very end of codegen. + std::vector> DeferredDiags; + public: CodeGenModule(ASTContext &C, const HeaderSearchOptions &headersearchopts, const PreprocessorOptions &ppopts, Index: clang/lib/CodeGen/CodeGenModule.cpp === --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -497,6 +497,16 @@ EmitVersionIdentMetadata(); EmitTargetMetadata(); + + // Emit any deferred diagnostics gathered during codegen. We didn't emit them + // when we first discovered them because that would have halted codegen, + // preventing us from gathering other deferred diags. + for (const PartialDiagnosticAt &DiagAt : DeferredDiags) { +SourceLocation Loc = DiagAt.first; +const PartialDiagnostic &PD = DiagAt.second; +DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID())); +PD.Emit(Builder); + } } void CodeGenModule::UpdateCompletedType(const TagDecl *TD) { @@ -2872,6 +2882,33 @@ llvm::GlobalValue *GV) { const auto *D = cast(GD.getDecl()); + // Emit this function's deferred diagnostics, if none of them are errors. If + // any of them are errors, don't codegen the function, but also don't emit any + // of the diagnostics just yet. Emitting an error during codegen stops + // further codegen, and we want to display as many deferred diags as possible. + // We'll emit the now twice-deferred diags at the very end of codegen. + // + // (If a function has both error and non-error diags, we don't emit the + // non-error diags here, because order can be significant, e.g. with notes + // that follow errors.) + auto Diags = D->takeDeferredDiags(); + bool HasError = llvm::any_of(Diags, [this](const PartialDiagnosticAt &PDAt) { +return getDiags().getDiagnosticLevel(PDAt.second.getDiagID(), PDAt.first) >= + DiagnosticsEngine::Error; + }); + if (HasError) { +DeferredDiags.insert(DeferredDiags.end(), + std::make_move_iterator(Diags.begin()), + std::make_move_iterator(Diags.end())); +return; + } + for (PartialDiagnosticAt &PDAt : Diags) { +const SourceLocation &Loc = PDAt.first; +const PartialDiagnostic &PD = PDAt.second; +DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID())); +PD.Emit(Builder); + } + // Compute the function info and LLVM type. const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD); llvm::FunctionType *Ty = getTypes().GetFunctionType(FI); Index: clang/lib/AST/Decl.cpp === --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -3446,6 +3446,20 @@ return 0; } +void FunctionDecl::addDeferredDiag(PartialDiagnosticAt PD) { + getASTContext().getDeferredDiags()[this].push_back(std::move(PD)); +} + +std::vector FunctionDecl::takeDeferredDiags() const { + auto &DD = getASTContext().getDeferredDiags(); + auto It = DD.find(this); + if (It == DD.end()) +return {}; + auto Ret = std::move(It->second); + DD.erase(It); + return Ret; +} + //===--===// // FieldDecl Implementation //===--===// Index: clang/include/clang/AST/Decl.h === --- clang/include/clang/AST/Decl.h +++ clang/include/clang/AST/Decl.h @@ -2271,6 +2271,14 @@ /// returns 0. unsigned getMemoryFunctionKind() const; + /// Add a diagnostic to be emitted if and when this function is codegen'ed. + void addDeferredDiag(PartialDiagnosticAt PD); + + /// Gets this object's list of deferred diagnostics, if there are any. + /// + /// Although this is logically const, it clears our list of deferred diags. + std::vector takeDeferredDiags() const; + // Implement isa/cast/dyncast/etc. static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classofKind(Kind K) { Index: clang/include/clang/AST/ASTContext.h === --- clang/include/clang/AST/ASTContext.h +++ clang/include/clang/AST/ASTContext.h @@ -324,6 +324,12 @@
Re: [PATCH] D23241: Add the notion of deferred diagnostics.
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D23241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D23241: Add the notion of deferred diagnostics.
This revision was automatically updated to reflect the committed changes. Closed by commit rL278735: Add the notion of deferred diagnostics. (authored by jlebar). Changed prior to commit: https://reviews.llvm.org/D23241?vs=68063&id=68073#toc Repository: rL LLVM https://reviews.llvm.org/D23241 Files: cfe/trunk/include/clang/AST/ASTContext.h cfe/trunk/include/clang/AST/Decl.h cfe/trunk/lib/AST/Decl.cpp cfe/trunk/lib/CodeGen/CodeGenModule.cpp cfe/trunk/lib/CodeGen/CodeGenModule.h Index: cfe/trunk/lib/AST/Decl.cpp === --- cfe/trunk/lib/AST/Decl.cpp +++ cfe/trunk/lib/AST/Decl.cpp @@ -3446,6 +3446,20 @@ return 0; } +void FunctionDecl::addDeferredDiag(PartialDiagnosticAt PD) { + getASTContext().getDeferredDiags()[this].push_back(std::move(PD)); +} + +std::vector FunctionDecl::takeDeferredDiags() const { + auto &DD = getASTContext().getDeferredDiags(); + auto It = DD.find(this); + if (It == DD.end()) +return {}; + auto Ret = std::move(It->second); + DD.erase(It); + return Ret; +} + //===--===// // FieldDecl Implementation //===--===// Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp === --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp @@ -497,6 +497,16 @@ EmitVersionIdentMetadata(); EmitTargetMetadata(); + + // Emit any deferred diagnostics gathered during codegen. We didn't emit them + // when we first discovered them because that would have halted codegen, + // preventing us from gathering other deferred diags. + for (const PartialDiagnosticAt &DiagAt : DeferredDiags) { +SourceLocation Loc = DiagAt.first; +const PartialDiagnostic &PD = DiagAt.second; +DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID())); +PD.Emit(Builder); + } } void CodeGenModule::UpdateCompletedType(const TagDecl *TD) { @@ -2872,6 +2882,33 @@ llvm::GlobalValue *GV) { const auto *D = cast(GD.getDecl()); + // Emit this function's deferred diagnostics, if none of them are errors. If + // any of them are errors, don't codegen the function, but also don't emit any + // of the diagnostics just yet. Emitting an error during codegen stops + // further codegen, and we want to display as many deferred diags as possible. + // We'll emit the now twice-deferred diags at the very end of codegen. + // + // (If a function has both error and non-error diags, we don't emit the + // non-error diags here, because order can be significant, e.g. with notes + // that follow errors.) + auto Diags = D->takeDeferredDiags(); + bool HasError = llvm::any_of(Diags, [this](const PartialDiagnosticAt &PDAt) { +return getDiags().getDiagnosticLevel(PDAt.second.getDiagID(), PDAt.first) >= + DiagnosticsEngine::Error; + }); + if (HasError) { +DeferredDiags.insert(DeferredDiags.end(), + std::make_move_iterator(Diags.begin()), + std::make_move_iterator(Diags.end())); +return; + } + for (PartialDiagnosticAt &PDAt : Diags) { +const SourceLocation &Loc = PDAt.first; +const PartialDiagnostic &PD = PDAt.second; +DiagnosticBuilder Builder(getDiags().Report(Loc, PD.getDiagID())); +PD.Emit(Builder); + } + // Compute the function info and LLVM type. const CGFunctionInfo &FI = getTypes().arrangeGlobalDeclaration(GD); llvm::FunctionType *Ty = getTypes().GetFunctionType(FI); Index: cfe/trunk/lib/CodeGen/CodeGenModule.h === --- cfe/trunk/lib/CodeGen/CodeGenModule.h +++ cfe/trunk/lib/CodeGen/CodeGenModule.h @@ -490,6 +490,10 @@ /// MDNodes. llvm::DenseMap MetadataIdMap; + /// Diags gathered from FunctionDecl::takeDeferredDiags(). Emitted at the + /// very end of codegen. + std::vector> DeferredDiags; + public: CodeGenModule(ASTContext &C, const HeaderSearchOptions &headersearchopts, const PreprocessorOptions &ppopts, Index: cfe/trunk/include/clang/AST/Decl.h === --- cfe/trunk/include/clang/AST/Decl.h +++ cfe/trunk/include/clang/AST/Decl.h @@ -2271,6 +2271,14 @@ /// returns 0. unsigned getMemoryFunctionKind() const; + /// Add a diagnostic to be emitted if and when this function is codegen'ed. + void addDeferredDiag(PartialDiagnosticAt PD); + + /// Gets this object's list of deferred diagnostics, if there are any. + /// + /// Although this is logically const, it clears our list of deferred diags. + std::vector takeDeferredDiags() const; + // Implement isa/cast/dyncast/etc. static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classofKind(Kind