On Mar 4, 2014, at 9:04 , Jordan Rose <[email protected]> wrote:
> > 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. r202864.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
