[APIDigester] Provide a category when serializing diagnostics representing API or ABI breakage

Author: owenv

include/swift/AST/DiagnosticConsumer.h

@@ -45,6 +45,7 @@ struct DiagnosticInfo {
   DiagnosticKind Kind;
   StringRef FormatString;
   ArrayRef<DiagnosticArgument> FormatArgs;
+  StringRef Category;
 
   /// Only used when directing diagnostics to different outputs.
   /// In batch mode a diagnostic may be
@@ -85,13 +86,13 @@ struct DiagnosticInfo {
 
   DiagnosticInfo(DiagID ID, SourceLoc Loc, DiagnosticKind Kind,
                  StringRef FormatString,
-                 ArrayRef<DiagnosticArgument> FormatArgs,
+                 ArrayRef<DiagnosticArgument> FormatArgs, StringRef Category,
                  SourceLoc BufferIndirectlyCausingDiagnostic,
                  ArrayRef<DiagnosticInfo *> ChildDiagnosticInfo,
                  ArrayRef<CharSourceRange> Ranges, ArrayRef<FixIt> FixIts,
                  bool IsChildNote)
       : ID(ID), Loc(Loc), Kind(Kind), FormatString(FormatString),
-        FormatArgs(FormatArgs),
+        FormatArgs(FormatArgs), Category(Category),
         BufferIndirectlyCausingDiagnostic(BufferIndirectlyCausingDiagnostic),
         ChildDiagnosticInfo(ChildDiagnosticInfo), Ranges(Ranges),
         FixIts(FixIts), IsChildNote(IsChildNote) {}

include/swift/AST/DiagnosticEngine.h

@@ -1013,6 +1013,10 @@ namespace swift {
     /// option.
     bool isDiagnosticPointsToFirstBadToken(DiagID id) const;
 
+    /// \returns true if the diagnostic is an API digester API or ABI breakage
+    /// diagnostic.
+    bool isAPIDigesterBreakageDiagnostic(DiagID id) const;
+
     /// \returns true if any diagnostic consumer gave an error while invoking
     //// \c finishProcessing.
     bool finishProcessing();

include/swift/AST/DiagnosticsModuleDiffer.def

@@ -20,73 +20,73 @@
 #define DEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"
 
-ERROR(generic_sig_change,none,"%0 has generic signature change from %1 to %2", (StringRef, StringRef, StringRef))
+ERROR(generic_sig_change,APIDigesterBreakage,"%0 has generic signature change from %1 to %2", (StringRef, StringRef, StringRef))
 
-ERROR(raw_type_change,none,"%0(%1) is now %2 representable", (StringRef, StringRef, StringRef))
+ERROR(raw_type_change,APIDigesterBreakage,"%0(%1) is now %2 representable", (StringRef, StringRef, StringRef))
 
-ERROR(removed_decl,none,"%0 has been removed%select{| (deprecated)}1", (StringRef, bool))
+ERROR(removed_decl,APIDigesterBreakage,"%0 has been removed%select{| (deprecated)}1", (StringRef, bool))
 
-ERROR(moved_decl,none,"%0 has been moved to %1", (StringRef, StringRef))
+ERROR(moved_decl,APIDigesterBreakage,"%0 has been moved to %1", (StringRef, StringRef))
 
-ERROR(renamed_decl,none,"%0 has been renamed to %1", (StringRef, StringRef))
+ERROR(renamed_decl,APIDigesterBreakage,"%0 has been renamed to %1", (StringRef, StringRef))
 
-ERROR(decl_type_change,none,"%0 has %1 type change from %2 to %3", (StringRef, StringRef, StringRef, StringRef))
+ERROR(decl_type_change,APIDigesterBreakage,"%0 has %1 type change from %2 to %3", (StringRef, StringRef, StringRef, StringRef))
 
-ERROR(decl_attr_change,none,"%0 changes from %1 to %2", (StringRef, StringRef, StringRef))
+ERROR(decl_attr_change,APIDigesterBreakage,"%0 changes from %1 to %2", (StringRef, StringRef, StringRef))
 
-ERROR(decl_new_attr,none,"%0 is now %1", (StringRef, StringRef))
+ERROR(decl_new_attr,APIDigesterBreakage,"%0 is now %1", (StringRef, StringRef))
 
-ERROR(decl_reorder,none,"%0 in a non-resilient type changes position from %1 to %2", (StringRef, unsigned, unsigned))
+ERROR(decl_reorder,APIDigesterBreakage,"%0 in a non-resilient type changes position from %1 to %2", (StringRef, unsigned, unsigned))
 
-ERROR(decl_added,none,"%0 is added to a non-resilient type", (StringRef))
+ERROR(decl_added,APIDigesterBreakage,"%0 is added to a non-resilient type", (StringRef))
 
-ERROR(var_has_fixed_order_change,none,"%0 is %select{now|no longer}1 a stored property", (StringRef, bool))
+ERROR(var_has_fixed_order_change,APIDigesterBreakage,"%0 is %select{now|no longer}1 a stored property", (StringRef, bool))
 
-ERROR(func_has_fixed_order_change,none,"%0 is %select{now|no longer}1 a non-final instance function", (StringRef, bool))
+ERROR(func_has_fixed_order_change,APIDigesterBreakage,"%0 is %select{now|no longer}1 a non-final instance function", (StringRef, bool))
 
-ERROR(default_arg_removed,none,"%0 has removed default argument from %1", (StringRef, StringRef))
+ERROR(default_arg_removed,APIDigesterBreakage,"%0 has removed default argument from %1", (StringRef, StringRef))
 
-ERROR(conformance_removed,none,"%0 has removed %select{conformance to|inherited protocol}2 %1", (StringRef, StringRef, bool))
+ERROR(conformance_removed,APIDigesterBreakage,"%0 has removed %select{conformance to|inherited protocol}2 %1", (StringRef, StringRef, bool))
 
-ERROR(conformance_added,none,"%0 has added inherited protocol %1", (StringRef, StringRef))
+ERROR(conformance_added,APIDigesterBreakage,"%0 has added inherited protocol %1", (StringRef, StringRef))
 
-ERROR(existing_conformance_added,none,"%0 has added a conformance to an existing protocol %1", (StringRef, StringRef))
+ERROR(existing_conformance_added,APIDigesterBreakage,"%0 has added a conformance to an existing protocol %1", (StringRef, StringRef))
 
-ERROR(default_associated_type_removed,none,"%0 has removed default type %1", (StringRef, StringRef))
+ERROR(default_associated_type_removed,APIDigesterBreakage,"%0 has removed default type %1", (StringRef, StringRef))
 
-ERROR(protocol_req_added,none,"%0 has been added as a protocol requirement", (StringRef))
+ERROR(protocol_req_added,APIDigesterBreakage,"%0 has been added as a protocol requirement", (StringRef))
 
-ERROR(super_class_removed,none,"%0 has removed its super class %1", (StringRef, StringRef))
+ERROR(super_class_removed,APIDigesterBreakage,"%0 has removed its super class %1", (StringRef, StringRef))
 
-ERROR(super_class_changed,none,"%0 has changed its super class from %1 to %2", (StringRef, StringRef, StringRef))
+ERROR(super_class_changed,APIDigesterBreakage,"%0 has changed its super class from %1 to %2", (StringRef, StringRef, StringRef))
 
-ERROR(decl_kind_changed,none,"%0 has been changed to a %1", (StringRef, StringRef))
+ERROR(decl_kind_changed,APIDigesterBreakage,"%0 has been changed to a %1", (StringRef, StringRef))
 
-ERROR(optional_req_changed,none,"%0 is %select{now|no longer}1 an optional requirement", (StringRef, bool))
+ERROR(optional_req_changed,APIDigesterBreakage,"%0 is %select{now|no longer}1 an optional requirement", (StringRef, bool))
 
-ERROR(no_longer_open,none,"%0 is no longer open for subclassing", (StringRef))
+ERROR(no_longer_open,APIDigesterBreakage,"%0 is no longer open for subclassing", (StringRef))
 
-ERROR(func_type_escaping_changed,none,"%0 has %select{removed|added}2 @escaping in %1", (StringRef, StringRef, bool))
+ERROR(func_type_escaping_changed,APIDigesterBreakage,"%0 has %select{removed|added}2 @escaping in %1", (StringRef, StringRef, bool))
 
-ERROR(func_self_access_change,none,"%0 has self access kind changing from %1 to %2", (StringRef, StringRef, StringRef))
+ERROR(func_self_access_change,APIDigesterBreakage,"%0 has self access kind changing from %1 to %2", (StringRef, StringRef, StringRef))
 
-ERROR(param_ownership_change,none,"%0 has %1 changing from %2 to %3", (StringRef, StringRef, StringRef, StringRef))
+ERROR(param_ownership_change,APIDigesterBreakage,"%0 has %1 changing from %2 to %3", (StringRef, StringRef, StringRef, StringRef))
 
-ERROR(type_witness_change,none,"%0 has type witness type for %1 changing from %2 to %3", (StringRef, StringRef, StringRef, StringRef))
+ERROR(type_witness_change,APIDigesterBreakage,"%0 has type witness type for %1 changing from %2 to %3", (StringRef, StringRef, StringRef, StringRef))
 
-ERROR(decl_new_witness_table_entry,none,"%0 now requires%select{| no}1 new witness table entry", (StringRef, bool))
+ERROR(decl_new_witness_table_entry,APIDigesterBreakage,"%0 now requires%select{| no}1 new witness table entry", (StringRef, bool))
 
-ERROR(new_decl_without_intro,none,"%0 is a new API without @available attribute", (StringRef))
+ERROR(new_decl_without_intro,APIDigesterBreakage,"%0 is a new API without @available attribute", (StringRef))
 
-ERROR(objc_name_change,none,"%0 has ObjC name change from %1 to %2", (StringRef, StringRef, StringRef))
+ERROR(objc_name_change,APIDigesterBreakage,"%0 has ObjC name change from %1 to %2", (StringRef, StringRef, StringRef))
 
-ERROR(desig_init_added,none,"%0 has been added as a designated initializer to an open class", (StringRef))
+ERROR(desig_init_added,APIDigesterBreakage,"%0 has been added as a designated initializer to an open class", (StringRef))
 
-ERROR(added_invisible_designated_init,none,"%0 has new designated initializers that are not visible to clients", (StringRef))
+ERROR(added_invisible_designated_init,APIDigesterBreakage,"%0 has new designated initializers that are not visible to clients", (StringRef))
 
-ERROR(not_inheriting_convenience_inits,none,"%0 no longer inherits convenience inits from its superclass", (StringRef))
+ERROR(not_inheriting_convenience_inits,APIDigesterBreakage,"%0 no longer inherits convenience inits from its superclass", (StringRef))
 
-ERROR(enum_case_added,none,"%0 has been added as a new enum case", (StringRef))
+ERROR(enum_case_added,APIDigesterBreakage,"%0 has been added as a new enum case", (StringRef))
 
 WARNING(cannot_read_allowlist,none,"cannot read breakage allowlist at '%0'", (StringRef))
 

lib/AST/DiagnosticEngine.cpp

@@ -52,19 +52,25 @@ enum class DiagnosticOptions {
 
   /// After a fatal error subsequent diagnostics are suppressed.
   Fatal,
+
+  /// An API or ABI breakage diagnostic emitted by the API digester.
+  APIDigesterBreakage,
 };
 struct StoredDiagnosticInfo {
   DiagnosticKind kind : 2;
   bool pointsToFirstBadToken : 1;
   bool isFatal : 1;
+  bool isAPIDigesterBreakage : 1;
 
   constexpr StoredDiagnosticInfo(DiagnosticKind k, bool firstBadToken,
-                                 bool fatal)
-      : kind(k), pointsToFirstBadToken(firstBadToken), isFatal(fatal) {}
+                                 bool fatal, bool isAPIDigesterBreakage)
+      : kind(k), pointsToFirstBadToken(firstBadToken), isFatal(fatal),
+        isAPIDigesterBreakage(isAPIDigesterBreakage) {}
   constexpr StoredDiagnosticInfo(DiagnosticKind k, DiagnosticOptions opts)
       : StoredDiagnosticInfo(k,
                              opts == DiagnosticOptions::PointsToFirstBadToken,
-                             opts == DiagnosticOptions::Fatal) {}
+                             opts == DiagnosticOptions::Fatal,
+                             opts == DiagnosticOptions::APIDigesterBreakage) {}
 };
 
 // Reproduce the DiagIDs, as we want both the size and access to the raw ids
@@ -316,6 +322,10 @@ bool DiagnosticEngine::isDiagnosticPointsToFirstBadToken(DiagID ID) const {
   return storedDiagnosticInfos[(unsigned) ID].pointsToFirstBadToken;
 }
 
+bool DiagnosticEngine::isAPIDigesterBreakageDiagnostic(DiagID ID) const {
+  return storedDiagnosticInfos[(unsigned)ID].isAPIDigesterBreakage;
+}
+
 bool DiagnosticEngine::finishProcessing() {
   bool hadError = false;
   for (auto &Consumer : Consumers) {
@@ -1092,11 +1102,17 @@ DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) {
     }
   }
 
+  // Currently, only API digester diagnostics are assigned a category.
+  StringRef Category;
+  if (isAPIDigesterBreakageDiagnostic(diagnostic.getID()))
+    Category = "api-digester-breaking-change";
+
   return DiagnosticInfo(
       diagnostic.getID(), loc, toDiagnosticKind(behavior),
       diagnosticStringFor(diagnostic.getID(), getPrintDiagnosticNames()),
-      diagnostic.getArgs(), getDefaultDiagnosticLoc(), /*child note info*/ {},
-      diagnostic.getRanges(), diagnostic.getFixIts(), diagnostic.isChildNote());
+      diagnostic.getArgs(), Category, getDefaultDiagnosticLoc(),
+      /*child note info*/ {}, diagnostic.getRanges(), diagnostic.getFixIts(),
+      diagnostic.isChildNote());
 }
 
 void DiagnosticEngine::emitDiagnostic(const Diagnostic &diagnostic) {

lib/Frontend/SerializedDiagnosticConsumer.cpp

@@ -84,6 +84,9 @@ struct SharedState : llvm::RefCountedBase<SharedState> {
   /// The collection of files used.
   llvm::DenseMap<const char *, unsigned> Files;
 
+  /// The collection of categories used.
+  llvm::DenseMap<const char *, unsigned> Categories;
+
   using DiagFlagsTy =
       llvm::DenseMap<const void *, std::pair<unsigned, StringRef>>;
 
@@ -187,6 +190,9 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
   // Record identifier for the file.
   unsigned getEmitFile(StringRef Filename);
 
+  // Record identifier for the category.
+  unsigned getEmitCategory(StringRef Category);
+
   /// Add a source location to a record.
   void addLocToRecord(SourceLoc Loc,
                       SourceManager &SM,
@@ -235,6 +241,23 @@ unsigned SerializedDiagnosticConsumer::getEmitFile(StringRef Filename) {
   return entry;
 }
 
+unsigned SerializedDiagnosticConsumer::getEmitCategory(StringRef Category) {
+  unsigned &entry = State->Categories[Category.data()];
+  if (entry)
+    return entry;
+
+  // Lazily generate the record for the category.
+  entry = State->Categories.size();
+  RecordData Record;
+  Record.push_back(RECORD_CATEGORY);
+  Record.push_back(entry);
+  Record.push_back(Category.size());
+  State->Stream.EmitRecordWithBlob(State->Abbrevs.get(RECORD_CATEGORY), Record,
+                                   Category.data());
+
+  return entry;
+}
+
 void SerializedDiagnosticConsumer::addLocToRecord(SourceLoc Loc,
                                                   SourceManager &SM,
                                                   StringRef Filename,
@@ -469,8 +492,13 @@ emitDiagnosticMessage(SourceManager &SM,
   Record.push_back(getDiagnosticLevel(Kind));
   addLocToRecord(Loc, SM, filename, Record);
 
-  // FIXME: Swift diagnostics currently have no category.
-  Record.push_back(0);
+  // Emit the category.
+  if (!Info.Category.empty()) {
+    Record.push_back(getEmitCategory(Info.Category));
+  } else {
+    Record.push_back(0);
+  }
+
   // FIXME: Swift diagnostics currently have no flags.
   Record.push_back(0);
 

lib/FrontendTool/FrontendTool.cpp

@@ -1973,9 +1973,10 @@ int swift::performFrontend(ArrayRef<const char *> Args,
     DiagnosticInfo errorInfo(
         DiagID(0), SourceLoc(), DiagnosticKind::Error,
         "fatal error encountered during compilation; " SWIFT_BUG_REPORT_MESSAGE,
-        {}, SourceLoc(), {}, {}, {}, false);
+        {}, StringRef(), SourceLoc(), {}, {}, {}, false);
     DiagnosticInfo noteInfo(DiagID(0), SourceLoc(), DiagnosticKind::Note,
-                            reason, {}, SourceLoc(), {}, {}, {}, false);
+                            reason, {}, StringRef(), SourceLoc(), {}, {}, {},
+                            false);
     PDC.handleDiagnostic(dummyMgr, errorInfo);
     PDC.handleDiagnostic(dummyMgr, noteInfo);
     if (shouldCrash)

test/api-digester/serialized-diagnostics.swift

@@ -0,0 +1,10 @@
+// REQUIRES: VENDOR=apple
+
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -disable-objc-attr-requires-foundation-module -emit-module -o %t/color.swiftmodule %S/Inputs/cake_baseline/color.swift -parse-as-library -enable-library-evolution %clang-importer-sdk-nosource -module-name color
+// RUN: not %api-digester -diagnose-sdk -serialize-diagnostics-path %t/result.dia -empty-baseline -I %t %clang-importer-sdk-nosource -module color -abi
+// RUN: c-index-test -read-diagnostics %t/result.dia 2>&1 | %FileCheck %s -check-prefix CHECK-DIA
+
+// Ensure the 'api-digester-breaking-change' category is included in the serialized diagnostics file.
+// CHECK-DIA: error: ABI breakage: enum Color is a new API without @available attribute [] [api-digester-breaking-change]

unittests/AST/DiagnosticConsumerTests.cpp

@@ -54,6 +54,7 @@ namespace {
   DiagnosticInfo testDiagnosticInfo(SourceLoc Loc, const char *Message,
                                     DiagnosticKind Kind) {
     return DiagnosticInfo(DiagID(0), Loc, Kind, Message, /*args*/ {},
+                          /*category*/ StringRef(),
                           /*indirectBuffer*/ SourceLoc(), /*childInfo*/ {},
                           /*ranges*/ {}, /*fixIts*/ {}, /*isChild*/ false);
   }