r202733 - Serialized diagnostic severity levels should be stable.
Jordan Rose
jordan_rose at apple.com
Tue Mar 4 09:52:05 PST 2014
On Mar 4, 2014, at 9:04 , Jordan Rose <jordan_rose at apple.com> wrote:
>
> On Mar 4, 2014, at 2:11 , Timur Iskhodzhanov <timurrrr at google.com> wrote:
>
>> 2014-03-03 22:29 GMT+04:00 Jordan Rose <jordan_rose at apple.com>:
>>> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140304/8136a37d/attachment.html>
More information about the cfe-commits
mailing list