From 5b9268f26ac53a74d2c504279fe577d988d5615d Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 20 Dec 2012 02:22:15 +0000 Subject: [PATCH] Fix code that attempted to produce a diagnostic with one DiagnosticEngine, then produce a note for that diagnostic either with a different DiagnosticEngine or after calling DiagnosticEngine::Reset(). That didn't make any sense, and did the wrong thing if the original diagnostic was suppressed. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@170636 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/ASTImporter.h | 3 +++ include/clang/Basic/Diagnostic.h | 6 ++++++ include/clang/Basic/DiagnosticASTKinds.td | 3 ++- lib/AST/ASTImporter.cpp | 22 ++++++++++++++++++++-- lib/Basic/Diagnostic.cpp | 6 +----- lib/Driver/Driver.cpp | 9 ++++++--- test/ASTMerge/Inputs/class1.cpp | 4 ++++ test/ASTMerge/Inputs/class2.cpp | 4 ++++ test/ASTMerge/class.cpp | 5 +++++ test/Misc/warning-flags.c | 3 +-- 10 files changed, 52 insertions(+), 13 deletions(-) diff --git a/include/clang/AST/ASTImporter.h b/include/clang/AST/ASTImporter.h index 46a9881039..1672ab22a3 100644 --- a/include/clang/AST/ASTImporter.h +++ b/include/clang/AST/ASTImporter.h @@ -48,6 +48,9 @@ namespace clang { /// \brief Whether to perform a minimal import. bool Minimal; + + /// \brief Whether the last diagnostic came from the "from" context. + bool LastDiagFromFrom; /// \brief Mapping from the already-imported types in the "from" context /// to the corresponding types in the "to" context. diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index 5e9395cae0..85e2cf5c7a 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -607,6 +607,12 @@ public: ArgToStringCookie = Cookie; } + /// \brief Note that the prior diagnostic was emitted by some other + /// \c DiagnosticsEngine, and we may be attaching a note to that diagnostic. + void notePriorDiagnosticFrom(const DiagnosticsEngine &Other) { + LastDiagLevel = Other.LastDiagLevel; + } + /// \brief Reset the state of the diagnostic object to its initial /// configuration. void Reset(); diff --git a/include/clang/Basic/DiagnosticASTKinds.td b/include/clang/Basic/DiagnosticASTKinds.td index d869c9983b..f3606b8ec4 100644 --- a/include/clang/Basic/DiagnosticASTKinds.td +++ b/include/clang/Basic/DiagnosticASTKinds.td @@ -135,7 +135,8 @@ def err_odr_function_type_inconsistent : Error< "external function %0 declared with incompatible types in different " "translation units (%1 vs. %2)">; def warn_odr_tag_type_inconsistent : Warning< - "type %0 has incompatible definitions in different translation units">; + "type %0 has incompatible definitions in different translation units">, + InGroup>; def note_odr_tag_kind_here: Note< "%0 is a %select{struct|interface|union|class|enum}1 here">; def note_odr_field : Note<"field %0 has type %1 here">; diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index f288953df6..33935c3b32 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -206,12 +206,16 @@ namespace { /// \brief Whether to complain about failures. bool Complain; + /// \brief \c true if the last diagnostic came from C2. + bool LastDiagFromC2; + StructuralEquivalenceContext(ASTContext &C1, ASTContext &C2, llvm::DenseSet > &NonEquivalentDecls, bool StrictTypeSpelling = false, bool Complain = true) : C1(C1), C2(C2), NonEquivalentDecls(NonEquivalentDecls), - StrictTypeSpelling(StrictTypeSpelling), Complain(Complain) { } + StrictTypeSpelling(StrictTypeSpelling), Complain(Complain), + LastDiagFromC2(false) {} /// \brief Determine whether the two declarations are structurally /// equivalent. @@ -229,11 +233,17 @@ namespace { public: DiagnosticBuilder Diag1(SourceLocation Loc, unsigned DiagID) { assert(Complain && "Not allowed to complain"); + if (LastDiagFromC2) + C1.getDiagnostics().notePriorDiagnosticFrom(C2.getDiagnostics()); + LastDiagFromC2 = false; return C1.getDiagnostics().Report(Loc, DiagID); } DiagnosticBuilder Diag2(SourceLocation Loc, unsigned DiagID) { assert(Complain && "Not allowed to complain"); + if (!LastDiagFromC2) + C2.getDiagnostics().notePriorDiagnosticFrom(C1.getDiagnostics()); + LastDiagFromC2 = true; return C2.getDiagnostics().Report(Loc, DiagID); } }; @@ -4319,7 +4329,7 @@ ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager, bool MinimalImport) : ToContext(ToContext), FromContext(FromContext), ToFileManager(ToFileManager), FromFileManager(FromFileManager), - Minimal(MinimalImport) + Minimal(MinimalImport), LastDiagFromFrom(false) { ImportedDecls[FromContext.getTranslationUnitDecl()] = ToContext.getTranslationUnitDecl(); @@ -4822,10 +4832,18 @@ DeclarationName ASTImporter::HandleNameConflict(DeclarationName Name, } DiagnosticBuilder ASTImporter::ToDiag(SourceLocation Loc, unsigned DiagID) { + if (LastDiagFromFrom) + ToContext.getDiagnostics().notePriorDiagnosticFrom( + FromContext.getDiagnostics()); + LastDiagFromFrom = false; return ToContext.getDiagnostics().Report(Loc, DiagID); } DiagnosticBuilder ASTImporter::FromDiag(SourceLocation Loc, unsigned DiagID) { + if (!LastDiagFromFrom) + FromContext.getDiagnostics().notePriorDiagnosticFrom( + ToContext.getDiagnostics()); + LastDiagFromFrom = true; return FromContext.getDiagnostics().Report(Loc, DiagID); } diff --git a/lib/Basic/Diagnostic.cpp b/lib/Basic/Diagnostic.cpp index f5eec02138..70d3939150 100644 --- a/lib/Basic/Diagnostic.cpp +++ b/lib/Basic/Diagnostic.cpp @@ -108,11 +108,7 @@ void DiagnosticsEngine::Reset() { TrapNumUnrecoverableErrorsOccurred = 0; CurDiagID = ~0U; - // Set LastDiagLevel to an "unset" state. If we set it to 'Ignored', notes - // using a DiagnosticsEngine associated to a translation unit that follow - // diagnostics from a DiagnosticsEngine associated to anoter t.u. will not be - // displayed. - LastDiagLevel = (DiagnosticIDs::Level)-1; + LastDiagLevel = DiagnosticIDs::Ignored; DelayedDiagID = 0; // Clear state related to #pragma diagnostic. diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index d41bce5ba5..dcb3c5fd09 100644 --- a/lib/Driver/Driver.cpp +++ b/lib/Driver/Driver.cpp @@ -366,9 +366,12 @@ void Driver::generateCompilationDiagnostics(Compilation &C, C.PrintDiagnosticJob(OS, C.getJobs()); OS.flush(); - // Clear stale state and suppress tool output. + // Keep track of whether we produce any errors while trying to produce + // preprocessed sources. + DiagnosticErrorTrap Trap(Diags); + + // Suppress tool output. C.initCompilationForDiagnostics(); - Diags.Reset(); // Construct the list of inputs. InputList Inputs; @@ -430,7 +433,7 @@ void Driver::generateCompilationDiagnostics(Compilation &C, BuildJobs(C); // If there were errors building the compilation, quit now. - if (Diags.hasErrorOccurred()) { + if (Trap.hasErrorOccurred()) { Diag(clang::diag::note_drv_command_failed_diag_msg) << "Error generating preprocessed source(s)."; return; diff --git a/test/ASTMerge/Inputs/class1.cpp b/test/ASTMerge/Inputs/class1.cpp index b600cdb1fc..0cd6565f1a 100644 --- a/test/ASTMerge/Inputs/class1.cpp +++ b/test/ASTMerge/Inputs/class1.cpp @@ -13,3 +13,7 @@ struct C { C &operator=(C&); ~C(); }; + +enum E { + b = 1 +}; diff --git a/test/ASTMerge/Inputs/class2.cpp b/test/ASTMerge/Inputs/class2.cpp index fa38916f5e..5d5d9ca233 100644 --- a/test/ASTMerge/Inputs/class2.cpp +++ b/test/ASTMerge/Inputs/class2.cpp @@ -7,3 +7,7 @@ struct B : A { int foo(); }; +enum E { + a = 0, + b = 1 +}; diff --git a/test/ASTMerge/class.cpp b/test/ASTMerge/class.cpp index 114687f8d9..885b65e983 100644 --- a/test/ASTMerge/class.cpp +++ b/test/ASTMerge/class.cpp @@ -1,9 +1,14 @@ // RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/class1.cpp // RUN: %clang_cc1 -emit-pch -o %t.2.ast %S/Inputs/class2.cpp // RUN: %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 -Wno-odr | count 0 // CHECK: class1.cpp:5:8: warning: type 'B' has incompatible definitions in different translation units // CHECK: class1.cpp:6:9: note: field 'y' has type 'float' here // CHECK: class2.cpp:6:7: note: field 'y' has type 'int' here // FIXME: we should also complain about mismatched types on the method + +// CHECK: class1.cpp:17:6: warning: type 'E' has incompatible definitions in different translation units +// CHECK: class1.cpp:18:3: note: enumerator 'b' with value 1 here +// CHECK: class2.cpp:11:3: note: enumerator 'a' with value 0 here diff --git a/test/Misc/warning-flags.c b/test/Misc/warning-flags.c index c3f14bce31..3a8c61bed9 100644 --- a/test/Misc/warning-flags.c +++ b/test/Misc/warning-flags.c @@ -18,7 +18,7 @@ This test serves two purposes: The list of warnings below should NEVER grow. It should gradually shrink to 0. -CHECK: Warnings without flags (148): +CHECK: Warnings without flags (147): CHECK-NEXT: ext_delete_void_ptr_operand CHECK-NEXT: ext_enum_friend CHECK-NEXT: ext_expected_semi_decl_list @@ -106,7 +106,6 @@ CHECK-NEXT: warn_not_compound_assign CHECK-NEXT: warn_objc_property_copy_missing_on_block CHECK-NEXT: warn_objc_protocol_qualifier_missing_id CHECK-NEXT: warn_octal_escape_too_large -CHECK-NEXT: warn_odr_tag_type_inconsistent CHECK-NEXT: warn_on_superclass_use CHECK-NEXT: warn_param_default_argument_redefinition CHECK-NEXT: warn_partial_specs_not_deducible -- 2.40.0