[Serialized diagnostics] Encode category documentation URL in existing record

Serialized diagnostics support an optional category name, which can be
used to group diagnostics together. This grouping is most helpful when
a number of diagnostics are placed in a category that is documented
together. For example, Clang's warnings are placed into groups that are
each documented. However, any relationship between the category name and
such documentation is left to the client to establish.

This change introduces a convention and APIs to allow diagnostic
categories to be associated with a URL at which one can find
documentation for the diagnostics in that category. Rather than
outright extend the serialized diagnostics format with a new record
kind, we utilize some redundancy to extend the existing RECORD_CATEGORY
in a backward-compatible way. Specifically, for diagnostic categories
that have documentation URLs associated with them, the blob associated
with a RECORD_CATEGORY entry will be

    <category name>@<category URL>

The RECORD_CATEGORY already includes a "category name length" field,
which specifies the length of the category name. A correct reader
would already be ignoring anything beyond that that position in the
blob, so we get this extensibility for free: all of the extra
information goes into the blob, but the category name length field
still describes the length of the category name (i.e., the position of
the @). A correct reader (that hasn't been updated for this change)
will treat diagnostics files that provide these documentation URLs in
the same way as ones that don't, ignoring the URL.

Amusingly, the two readers I looked at (the one in Clang and a Swift
one we use for other tools) ignore the category name length field.
They still fail fairly gracefully: the category name ends up being the
whole blob, which is still human-readable and fairly reasonable.

This change teaches Clang's serialized diagnostic reader to properly
respect the category name length field and, when the blob matches the
description above, separately track the category documentation URL. It
adds a new libclang diagnostics API `clang_getDiagnosticCategoryURL()`
that provides this URL separately from the category.

c-index-test prints the category name -> URL mapping separate from the
diagnostic so we can test this functionality.

This commit does not teach Clang to encode documentation URLs in the
diagnostics it generates. That will come separately.

Author: DougGregor

clang/include/clang-c/CXDiagnostic.h

@@ -338,6 +338,14 @@ clang_getDiagnosticCategoryName(unsigned Category);
  */
 CINDEX_LINKAGE CXString clang_getDiagnosticCategoryText(CXDiagnostic);
 
+/**
+ * Retrieve the diagnostic category's URL for a given diagnostic.
+ *
+ * \returns The URL that provides additional documentation for the
+ * category of this diagnostic.
+ */
+CINDEX_LINKAGE CXString clang_getDiagnosticCategoryURL(CXDiagnostic);
+
 /**
  * Determine the number of source ranges associated with the given
  * diagnostic.

clang/include/clang/Frontend/SerializedDiagnosticReader.h

@@ -89,10 +89,21 @@ class SerializedDiagnosticReader {
   virtual std::error_code visitEndOfDiagnostic() { return {}; }
 
   /// Visit a category. This associates the category \c ID to a \c Name.
+  ///
+  /// This entrypoint has been superseded by the overload that follows, which
+  /// also takes a (possibly-empty) URL providing additional documentation for
+  /// the category.
   virtual std::error_code visitCategoryRecord(unsigned ID, StringRef Name) {
     return {};
   }
 
+  /// Visit a category. This associates the category \c ID to a \c Name with
+  /// a (possibly empty) URL.
+  virtual std::error_code visitCategoryRecord(unsigned ID, StringRef Name,
+                                              StringRef URL) {
+    return visitCategoryRecord(ID, Name);
+  }
+
   /// Visit a flag. This associates the flag's \c ID to a \c Name.
   virtual std::error_code visitDiagFlagRecord(unsigned ID, StringRef Name) {
     return {};

clang/lib/Frontend/SerializedDiagnosticReader.cpp

@@ -266,13 +266,28 @@ SerializedDiagnosticReader::readDiagnosticBlock(llvm::BitstreamCursor &Stream) {
       continue;
 
     switch ((RecordIDs)RecID) {
-    case RECORD_CATEGORY:
+    case RECORD_CATEGORY: {
       // A category has ID and name size.
       if (Record.size() != 2)
         return SDError::MalformedDiagnosticRecord;
-      if ((EC = visitCategoryRecord(Record[0], Blob)))
+
+      // Make sure the name size makes sense.
+      unsigned NameLen = Record[1];
+      if (NameLen > Blob.size())
+        return SDError::MalformedDiagnosticRecord;
+
+      StringRef Name = Blob.substr(0, NameLen);
+      StringRef URL;
+
+      // If the name didn't take up the full blob, and the character that
+      // follows it is an '@', the rest is the category URL.
+      if (NameLen < Blob.size() && Blob[NameLen] == '@')
+        URL = Blob.substr(NameLen + 1);
+
+      if ((EC = visitCategoryRecord(Record[0], Name, URL)))
         return EC;
       continue;
+    }
     case RECORD_DIAG:
       // A diagnostic has severity, location (4), category, flag, and message
       // size.

clang/tools/c-index-test/c-index-test.c

@@ -4823,9 +4823,10 @@ static void printDiagnosticSet(
     CXSourceLocation DiagLoc;
     CXDiagnostic D;
     CXFile File;
-    CXString FileName, DiagSpelling, DiagOption, DiagCat;
+    CXString FileName, DiagSpelling, DiagOption, DiagCat, DiagCatURL;
     unsigned line, column, offset;
-    const char *FileNameStr = 0, *DiagOptionStr = 0, *DiagCatStr = 0;
+    const char *FileNameStr = 0, *DiagOptionStr = 0, *DiagCatStr = 0,
+      *DiagCatURLStr = 0;
     const char *FileContents = 0;
 
     D = clang_getDiagnosticInSet(Diags, i);
@@ -4883,13 +4884,20 @@ static void printDiagnosticSet(
       fprintf(stderr, "%s\nEND CONTENTS OF FILE\n", FileContents);
     }
 
+    DiagCatURL = clang_getDiagnosticCategoryURL(D);
+    DiagCatURLStr = clang_getCString(DiagCatURL);
+    if (DiagCatURLStr && DiagCatStr && DiagCatURLStr[0]) {
+      fprintf(stderr, "[%s]: <%s>\n", DiagCatStr, DiagCatURLStr);
+    }
+
     /* Print subdiagnostics. */
     printDiagnosticSet(clang_getChildDiagnostics(D), indent+2, TopDiags);
 
     clang_disposeString(FileName);
     clang_disposeString(DiagSpelling);
     clang_disposeString(DiagOption);
     clang_disposeString(DiagCat);
+    clang_disposeString(DiagCatURL);
   }
 }
 

clang/tools/libclang/CIndexDiagnostic.cpp

@@ -70,6 +70,7 @@ class CXDiagnosticCustomNoteImpl : public CXDiagnosticImpl {
 
   unsigned getCategory() const override { return 0; }
   CXString getCategoryText() const override { return cxstring::createEmpty(); }
+  CXString getCategoryURL() const override { return cxstring::createEmpty(); }
 
   unsigned getNumRanges() const override { return 0; }
   CXSourceRange getRange(unsigned Range) const override {
@@ -431,7 +432,13 @@ CXString clang_getDiagnosticCategoryText(CXDiagnostic Diag) {
     return D->getCategoryText();
   return cxstring::createEmpty();
 }
-  
+
+CXString clang_getDiagnosticCategoryURL(CXDiagnostic Diag) {
+  if (CXDiagnosticImpl *D = static_cast<CXDiagnosticImpl *>(Diag))
+    return D->getCategoryURL();
+  return cxstring::createEmpty();
+}
+
 unsigned clang_getDiagnosticNumRanges(CXDiagnostic Diag) {
   if (CXDiagnosticImpl *D = static_cast<CXDiagnosticImpl *>(Diag))
     return D->getNumRanges();

clang/tools/libclang/CIndexDiagnostic.h

@@ -99,6 +99,9 @@ class CXDiagnosticImpl {
   /// Return the category string of the diagnostic.
   virtual CXString getCategoryText() const = 0;
 
+  /// Return the category URL of the diagnostic.
+  virtual CXString getCategoryURL() const = 0;
+
   /// Return the number of source ranges for the diagnostic.
   virtual unsigned getNumRanges() const = 0;
 
@@ -160,6 +163,9 @@ struct CXStoredDiagnostic : public CXDiagnosticImpl {
   /// Return the category string of the diagnostic.
   CXString getCategoryText() const override;
 
+  /// Return the category URL of the diagnostic.
+  CXString getCategoryURL() const override;
+
   /// Return the number of source ranges for the diagnostic.
   unsigned getNumRanges() const override;
 

clang/tools/libclang/CXLoadedDiagnostic.cpp

@@ -41,6 +41,7 @@ class CXLoadedDiagnosticSetImpl : public CXDiagnosticSetImpl {
 
   llvm::BumpPtrAllocator Alloc;
   Strings Categories;
+  Strings CategoryURLs;
   Strings WarningFlags;
   Strings FileNames;
 
@@ -126,6 +127,10 @@ CXString CXLoadedDiagnostic::getCategoryText() const {
   return cxstring::createDup(CategoryText);
 }
 
+CXString CXLoadedDiagnostic::getCategoryURL() const {
+  return cxstring::createDup(CategoryURL);
+}
+
 unsigned CXLoadedDiagnostic::getNumRanges() const {
   return Ranges.size();
 }
@@ -215,7 +220,8 @@ class DiagLoader : serialized_diags::SerializedDiagnosticReader {
   std::error_code visitStartOfDiagnostic() override;
   std::error_code visitEndOfDiagnostic() override;
 
-  std::error_code visitCategoryRecord(unsigned ID, StringRef Name) override;
+  std::error_code visitCategoryRecord(unsigned ID, StringRef Name,
+                                      StringRef URL) override;
 
   std::error_code visitDiagFlagRecord(unsigned ID, StringRef Name) override;
 
@@ -345,11 +351,13 @@ std::error_code DiagLoader::visitEndOfDiagnostic() {
   return std::error_code();
 }
 
-std::error_code DiagLoader::visitCategoryRecord(unsigned ID, StringRef Name) {
+std::error_code DiagLoader::visitCategoryRecord(unsigned ID, StringRef Name,
+                                                StringRef URL) {
   // FIXME: Why do we care about long strings?
   if (Name.size() > 65536)
     return reportInvalidFile("Out-of-bounds string in category");
   TopDiags->Categories[ID] = TopDiags->copyString(Name);
+  TopDiags->CategoryURLs[ID] = TopDiags->copyString(URL);
   return std::error_code();
 }
 
@@ -431,6 +439,7 @@ std::error_code DiagLoader::visitDiagnosticRecord(
   D.category = Category;
   D.DiagOption = Flag ? TopDiags->WarningFlags[Flag] : "";
   D.CategoryText = Category ? TopDiags->Categories[Category] : "";
+  D.CategoryURL = Category ? TopDiags->CategoryURLs[Category] : "";
   D.Spelling = TopDiags->copyString(Message);
   return std::error_code();
 }

clang/tools/libclang/CXLoadedDiagnostic.h

@@ -49,6 +49,9 @@ class CXLoadedDiagnostic : public CXDiagnosticImpl {
   /// Return the category string of the diagnostic.
   CXString getCategoryText() const override;
 
+  /// Return the category URL of the diagnostic.
+  CXString getCategoryURL() const override;
+
   /// Return the number of source ranges for the diagnostic.
   unsigned getNumRanges() const override;
 
@@ -89,6 +92,7 @@ class CXLoadedDiagnostic : public CXDiagnosticImpl {
   const char *Spelling;
   llvm::StringRef DiagOption;
   llvm::StringRef CategoryText;
+  llvm::StringRef CategoryURL;
   unsigned severity;
   unsigned category;
 };

clang/tools/libclang/CXStoredDiagnostic.cpp

@@ -78,6 +78,11 @@ CXString CXStoredDiagnostic::getCategoryText() const {
   return cxstring::createRef(DiagnosticIDs::getCategoryNameFromID(catID));
 }
 
+CXString CXStoredDiagnostic::getCategoryURL() const {
+  // Clang does not currently provide URLs for its own diagnostics.
+  return cxstring::createEmpty();
+}
+
 unsigned CXStoredDiagnostic::getNumRanges() const {
   if (Diag.getLocation().isInvalid())
     return 0;

clang/tools/libclang/libclang.map

@@ -339,6 +339,7 @@ LLVM_13 {
     clang_getDiagnosticCategory;
     clang_getDiagnosticCategoryName;
     clang_getDiagnosticCategoryText;
+    clang_getDiagnosticCategoryURL;
     clang_getDiagnosticFixIt;
     clang_getDiagnosticInSet;
     clang_getDiagnosticLocation;