On Mar 4, 2014, at 2:11 , Timur Iskhodzhanov <[email protected]> wrote:
> 2014-03-03 22:29 GMT+04:00 Jordan Rose <[email protected]>: >> Author: jrose >> Date: Mon Mar 3 12:29:52 2014 >> New Revision: 202733 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=202733&view=rev >> Log: >> Serialized diagnostic severity levels should be stable. >> >> Serialized diagnostics were accidentally using the AST diagnostic level >> values >> rather than a dedicated stable enum, so the addition of "remark" broke the >> reading of existing serialized diagnostics files. I've added a .dia file >> generated from Xcode 5's Clang to make sure we don't break this in the >> future. >> >> Added: >> cfe/trunk/test/Misc/Inputs/serialized-diags-stable.dia >> cfe/trunk/test/Misc/serialized-diags-stable.c >> Modified: >> cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h >> cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp >> cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp >> >> Modified: cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h?rev=202733&r1=202732&r2=202733&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h (original) >> +++ cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h Mon Mar >> 3 12:29:52 2014 >> @@ -46,6 +46,19 @@ enum RecordIDs { >> RECORD_LAST = RECORD_FIXIT >> }; >> >> +/// A stable version of DiagnosticIDs::Level. >> +/// >> +/// Do not change the order of values in this enum, and please increment the >> +/// serialized diagnostics version number when you add to it. >> +enum Level { >> + Ignored = 0, >> + Note, >> + Warning, >> + Error, >> + Fatal, >> + Remark >> +}; >> + >> /// \brief Returns a DiagnosticConsumer that serializes diagnostics to >> /// a bitcode file. >> /// >> >> Modified: cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp?rev=202733&r1=202732&r2=202733&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp (original) >> +++ cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp Mon Mar 3 >> 12:29:52 2014 >> @@ -174,7 +174,7 @@ private: >> const SourceManager &SM); >> >> /// \brief The version of the diagnostics file. >> - enum { Version = 1 }; >> + enum { Version = 2 }; >> >> /// \brief Language options, which can differ from one clone of this client >> /// to another. >> @@ -566,6 +566,21 @@ void SDiagsWriter::HandleDiagnostic(Diag >> &Info); >> } >> >> +static serialized_diags::Level getStableLevel(DiagnosticsEngine::Level >> Level) { >> + switch (Level) { >> +#define CASE(X) case DiagnosticsEngine::X: return serialized_diags::X; >> + CASE(Ignored) >> + CASE(Note) >> + CASE(Remark) >> + CASE(Warning) >> + CASE(Error) >> + CASE(Fatal) >> +#undef CASE >> + } >> + >> + llvm_unreachable("invalid diagnostic level"); >> +} >> + >> void SDiagsWriter::EmitDiagnosticMessage(SourceLocation Loc, >> PresumedLoc PLoc, >> DiagnosticsEngine::Level Level, >> @@ -579,7 +594,7 @@ void SDiagsWriter::EmitDiagnosticMessage >> // Emit the RECORD_DIAG record. >> Record.clear(); >> Record.push_back(RECORD_DIAG); >> - Record.push_back(Level); >> + Record.push_back(getStableLevel(Level)); >> AddLocToRecord(Loc, SM, PLoc, Record); >> >> if (const Diagnostic *Info = D.dyn_cast<const Diagnostic*>()) { >> >> Added: cfe/trunk/test/Misc/Inputs/serialized-diags-stable.dia >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/Inputs/serialized-diags-stable.dia?rev=202733&view=auto >> ============================================================================== >> Binary files cfe/trunk/test/Misc/Inputs/serialized-diags-stable.dia (added) >> and cfe/trunk/test/Misc/Inputs/serialized-diags-stable.dia Mon Mar 3 >> 12:29:52 2014 differ >> >> Added: cfe/trunk/test/Misc/serialized-diags-stable.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/serialized-diags-stable.c?rev=202733&view=auto >> ============================================================================== >> --- cfe/trunk/test/Misc/serialized-diags-stable.c (added) >> +++ cfe/trunk/test/Misc/serialized-diags-stable.c Mon Mar 3 12:29:52 2014 >> @@ -0,0 +1,20 @@ >> +// RUN: rm -f %t >> +// RUN: not %clang -Wall -fsyntax-only %s --serialize-diagnostics %t.dia > >> /dev/null 2>&1 >> +// RUN: c-index-test -read-diagnostics %t.dia 2>&1 | FileCheck %s >> + >> +// RUN: c-index-test -read-diagnostics >> %S/Inputs/serialized-diags-stable.dia 2>&1 | FileCheck %s >> + >> +int foo() { >> + // CHECK: serialized-diags-stable.c:[[@LINE+2]]:1: warning: control >> reaches end of non-void function [-Wreturn-type] [Semantic Issue] >> + // CHECK-NEXT: Number FIXITs = 0 >> +} >> + >> +// CHECK: serialized-diags-stable.c:[[@LINE+5]]:13: error: redefinition of >> 'bar' as different kind of symbol [] [Semantic Issue] >> +// CHECK-NEXT: Number FIXITs = 0 >> +// CHECK-NEXT: >> +-/Volumes/Lore/llvm-public/clang/test/Misc/serialized-diags-stable.c:[[@LINE+2]]:6: >> note: previous definition is here [] [] >> +// CHECK-NEXT: Number FIXITs = 0 >> +void bar() {} >> +typedef int bar; >> + >> + >> +// CHECK-LABEL: Number of diagnostics: 2 >> >> Modified: cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp?rev=202733&r1=202732&r2=202733&view=diff >> ============================================================================== >> --- cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp (original) >> +++ cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp Mon Mar 3 12:29:52 2014 >> @@ -67,13 +67,19 @@ CXLoadedDiagnostic::~CXLoadedDiagnostic( >> //===----------------------------------------------------------------------===// >> >> CXDiagnosticSeverity CXLoadedDiagnostic::getSeverity() const { >> - // FIXME: possibly refactor with logic in CXStoredDiagnostic. >> - switch (severity) { >> - case DiagnosticsEngine::Ignored: return CXDiagnostic_Ignored; >> - case DiagnosticsEngine::Note: return CXDiagnostic_Note; >> - case DiagnosticsEngine::Warning: return CXDiagnostic_Warning; >> - case DiagnosticsEngine::Error: return CXDiagnostic_Error; >> - case DiagnosticsEngine::Fatal: return CXDiagnostic_Fatal; >> + // FIXME: Fail more softly if the diagnostic level is unknown? >> + assert(severity == static_cast<serialized_diags::Level>(severity) && >> + "unknown serialized diagnostic level"); > > FYI gcc 4.8.2 warns on > tools/clang/tools/libclang/CXLoadedDiagnostic.cpp:71:67: error: > comparison between signed and unsigned integer expressions > [-Werror=sign-compare] > assert(severity == static_cast<serialized_diags::Level>(severity) Thanks, I didn't realize we had -Wsign-compare on anywhere. Will fix. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
