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