[clang][ExtractAPI] Add support C unions in non C++ parsing mode

daniel-grumberg · daniel-grumberg · commit 9ed6ab49d39d · 2024-01-11T13:31:59.000Z
Ensure that we generate correct symbol kinds and declaration fragments
for unions in C and Objective-C parsing modes.

rdar://120544091
diff --git a/clang/include/clang/ExtractAPI/API.h b/clang/include/clang/ExtractAPI/API.h
@@ -169,6 +169,7 @@ struct APIRecord {
     RK_Enum,
     RK_StructField,
     RK_Struct,
+    RK_UnionField,
     RK_Union,
     RK_StaticField,
     RK_CXXField,
@@ -478,37 +479,40 @@ struct EnumRecord : APIRecord {
 };
 
 /// This holds information associated with struct fields.
-struct StructFieldRecord : APIRecord {
-  StructFieldRecord(StringRef USR, StringRef Name, PresumedLoc Loc,
+struct RecordFieldRecord : APIRecord {
+  RecordFieldRecord(StringRef USR, StringRef Name, PresumedLoc Loc,
                     AvailabilitySet Availabilities, const DocComment &Comment,
                     DeclarationFragments Declaration,
-                    DeclarationFragments SubHeading, bool IsFromSystemHeader)
-      : APIRecord(RK_StructField, USR, Name, Loc, std::move(Availabilities),
+                    DeclarationFragments SubHeading, RecordKind Kind,
+                    bool IsFromSystemHeader)
+      : APIRecord(Kind, USR, Name, Loc, std::move(Availabilities),
                   LinkageInfo::none(), Comment, Declaration, SubHeading,
                   IsFromSystemHeader) {}
 
   static bool classof(const APIRecord *Record) {
-    return Record->getKind() == RK_StructField;
+    return Record->getKind() == RK_StructField ||
+           Record->getKind() == RK_UnionField;
   }
 
 private:
   virtual void anchor();
 };
 
 /// This holds information associated with structs.
-struct StructRecord : APIRecord {
-  SmallVector<std::unique_ptr<StructFieldRecord>> Fields;
+struct RecordRecord : APIRecord {
+  SmallVector<std::unique_ptr<RecordFieldRecord>> Fields;
 
-  StructRecord(StringRef USR, StringRef Name, PresumedLoc Loc,
+  RecordRecord(StringRef USR, StringRef Name, PresumedLoc Loc,
                AvailabilitySet Availabilities, const DocComment &Comment,
                DeclarationFragments Declaration,
-               DeclarationFragments SubHeading, bool IsFromSystemHeader)
-      : APIRecord(RK_Struct, USR, Name, Loc, std::move(Availabilities),
+               DeclarationFragments SubHeading, RecordKind Kind,
+               bool IsFromSystemHeader)
+      : APIRecord(Kind, USR, Name, Loc, std::move(Availabilities),
                   LinkageInfo::none(), Comment, Declaration, SubHeading,
                   IsFromSystemHeader) {}
 
   static bool classof(const APIRecord *Record) {
-    return Record->getKind() == RK_Struct;
+    return Record->getKind() == RK_Struct || Record->getKind() == RK_Union;
   }
 
 private:
@@ -1267,30 +1271,31 @@ class APISet {
                       DeclarationFragments Declaration,
                       DeclarationFragments SubHeading, bool IsFromSystemHeader);
 
-  /// Create and add a struct field record into the API set.
+  /// Create and add a record field record into the API set.
   ///
   /// Note: the caller is responsible for keeping the StringRef \p Name and
   /// \p USR alive. APISet::copyString provides a way to copy strings into
   /// APISet itself, and APISet::recordUSR(const Decl *D) is a helper method
   /// to generate the USR for \c D and keep it alive in APISet.
-  StructFieldRecord *
-  addStructField(StructRecord *Struct, StringRef Name, StringRef USR,
+  RecordFieldRecord *
+  addRecordField(RecordRecord *Record, StringRef Name, StringRef USR,
                  PresumedLoc Loc, AvailabilitySet Availability,
                  const DocComment &Comment, DeclarationFragments Declaration,
-                 DeclarationFragments SubHeading, bool IsFromSystemHeader);
+                 DeclarationFragments SubHeading, APIRecord::RecordKind Kind,
+                 bool IsFromSystemHeader);
 
-  /// Create and add a struct record into the API set.
+  /// Create and add a record record into the API set.
   ///
   /// Note: the caller is responsible for keeping the StringRef \p Name and
   /// \p USR alive. APISet::copyString provides a way to copy strings into
   /// APISet itself, and APISet::recordUSR(const Decl *D) is a helper method
   /// to generate the USR for \c D and keep it alive in APISet.
-  StructRecord *addStruct(StringRef Name, StringRef USR, PresumedLoc Loc,
+  RecordRecord *addRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
                           AvailabilitySet Availability,
                           const DocComment &Comment,
                           DeclarationFragments Declaration,
                           DeclarationFragments SubHeading,
-                          bool IsFromSystemHeader);
+                          APIRecord::RecordKind Kind, bool IsFromSystemHeader);
 
   StaticFieldRecord *
   addStaticField(StringRef Name, StringRef USR, PresumedLoc Loc,
@@ -1545,7 +1550,7 @@ class APISet {
     return GlobalVariableTemplatePartialSpecializations;
   }
   const RecordMap<EnumRecord> &getEnums() const { return Enums; }
-  const RecordMap<StructRecord> &getStructs() const { return Structs; }
+  const RecordMap<RecordRecord> &getRecords() const { return Records; }
   const RecordMap<CXXClassRecord> &getCXXClasses() const { return CXXClasses; }
   const RecordMap<CXXMethodTemplateRecord> &getCXXMethodTemplates() const {
     return CXXMethodTemplates;
@@ -1564,7 +1569,6 @@ class APISet {
   const RecordMap<CXXFieldTemplateRecord> &getCXXFieldTemplates() const {
     return CXXFieldTemplates;
   }
-  const RecordMap<ConceptRecord> &getConcepts() const { return Concepts; }
   const RecordMap<ClassTemplateRecord> &getClassTemplates() const {
     return ClassTemplates;
   }
@@ -1576,7 +1580,7 @@ class APISet {
   getClassTemplatePartialSpecializations() const {
     return ClassTemplatePartialSpecializations;
   }
-  const RecordMap<ConceptRecord> &getRecords() const { return Concepts; }
+  const RecordMap<ConceptRecord> &getConcepts() const { return Concepts; }
   const RecordMap<ObjCCategoryRecord> &getObjCCategories() const {
     return ObjCCategories;
   }
@@ -1642,7 +1646,7 @@ class APISet {
   RecordMap<ConceptRecord> Concepts;
   RecordMap<StaticFieldRecord> StaticFields;
   RecordMap<EnumRecord> Enums;
-  RecordMap<StructRecord> Structs;
+  RecordMap<RecordRecord> Records;
   RecordMap<CXXClassRecord> CXXClasses;
   RecordMap<CXXFieldRecord> CXXFields;
   RecordMap<CXXMethodRecord> CXXMethods;
diff --git a/clang/include/clang/ExtractAPI/DeclarationFragments.h b/clang/include/clang/ExtractAPI/DeclarationFragments.h
@@ -295,8 +295,9 @@ class DeclarationFragmentsBuilder {
   /// Build DeclarationFragments for a field declaration FieldDecl.
   static DeclarationFragments getFragmentsForField(const FieldDecl *);
 
-  /// Build DeclarationFragments for a struct record declaration RecordDecl.
-  static DeclarationFragments getFragmentsForStruct(const RecordDecl *);
+  /// Build DeclarationFragments for a struct/union record declaration
+  /// RecordDecl.
+  static DeclarationFragments getFragmentsForRecordDecl(const RecordDecl *);
 
   static DeclarationFragments getFragmentsForCXXClass(const CXXRecordDecl *);
 
diff --git a/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h b/clang/include/clang/ExtractAPI/ExtractAPIVisitor.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_EXTRACTAPI_EXTRACT_API_VISITOR_H
 #define LLVM_CLANG_EXTRACTAPI_EXTRACT_API_VISITOR_H
 
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/OperatorKinds.h"
@@ -128,9 +129,10 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
   void recordEnumConstants(EnumRecord *EnumRecord,
                            const EnumDecl::enumerator_range Constants);
 
-  /// Collect API information for the struct fields and associate with the
+  /// Collect API information for the record fields and associate with the
   /// parent struct.
-  void recordStructFields(StructRecord *StructRecord,
+  void recordRecordFields(RecordRecord *RecordRecord,
+                          APIRecord::RecordKind FieldKind,
                           const RecordDecl::field_range Fields);
 
   /// Collect API information for the Objective-C methods and associate with the
@@ -524,17 +526,25 @@ bool ExtractAPIVisitorBase<Derived>::VisitRecordDecl(const RecordDecl *Decl) {
 
   // Build declaration fragments and sub-heading for the struct.
   DeclarationFragments Declaration =
-      DeclarationFragmentsBuilder::getFragmentsForStruct(Decl);
+      DeclarationFragmentsBuilder::getFragmentsForRecordDecl(Decl);
   DeclarationFragments SubHeading =
       DeclarationFragmentsBuilder::getSubHeading(Decl);
 
-  StructRecord *StructRecord =
-      API.addStruct(Name, USR, Loc, AvailabilitySet(Decl), Comment, Declaration,
-                    SubHeading, isInSystemHeader(Decl));
+  auto RecordKind = APIRecord::RK_Struct;
+  auto FieldRecordKind = APIRecord::RK_StructField;
+
+  if (Decl->isUnion()) {
+    RecordKind = APIRecord::RK_Union;
+    FieldRecordKind = APIRecord::RK_UnionField;
+  }
+
+  RecordRecord *RecordRecord =
+      API.addRecord(Name, USR, Loc, AvailabilitySet(Decl), Comment, Declaration,
+                    SubHeading, RecordKind, isInSystemHeader(Decl));
 
   // Now collect information about the fields in this struct.
-  getDerivedExtractAPIVisitor().recordStructFields(StructRecord,
-                                                   Decl->fields());
+  getDerivedExtractAPIVisitor().recordRecordFields(
+      RecordRecord, FieldRecordKind, Decl->fields());
 
   return true;
 }
@@ -1055,8 +1065,8 @@ bool ExtractAPIVisitorBase<Derived>::VisitTypedefNameDecl(
           dyn_cast<ElaboratedType>(Decl->getUnderlyingType())) {
     if (const TagType *TagTy = dyn_cast<TagType>(ET->desugar())) {
       if (Decl->getName() == TagTy->getDecl()->getName()) {
-        if (TagTy->getDecl()->isStruct()) {
-          modifyRecords(API.getStructs(), Decl->getName());
+        if (isa<RecordDecl>(TagTy->getDecl())) {
+          modifyRecords(API.getRecords(), Decl->getName());
         }
         if (TagTy->getDecl()->isEnum()) {
           modifyRecords(API.getEnums(), Decl->getName());
@@ -1169,8 +1179,9 @@ void ExtractAPIVisitorBase<Derived>::recordEnumConstants(
 /// Collect API information for the struct fields and associate with the
 /// parent struct.
 template <typename Derived>
-void ExtractAPIVisitorBase<Derived>::recordStructFields(
-    StructRecord *StructRecord, const RecordDecl::field_range Fields) {
+void ExtractAPIVisitorBase<Derived>::recordRecordFields(
+    RecordRecord *RecordRecord, APIRecord::RecordKind FieldKind,
+    const RecordDecl::field_range Fields) {
   for (const auto *Field : Fields) {
     // Collect symbol information.
     StringRef Name = Field->getName();
@@ -1189,8 +1200,8 @@ void ExtractAPIVisitorBase<Derived>::recordStructFields(
     DeclarationFragments SubHeading =
         DeclarationFragmentsBuilder::getSubHeading(Field);
 
-    API.addStructField(StructRecord, Name, USR, Loc, AvailabilitySet(Field),
-                       Comment, Declaration, SubHeading,
+    API.addRecordField(RecordRecord, Name, USR, Loc, AvailabilitySet(Field),
+                       Comment, Declaration, SubHeading, FieldKind,
                        isInSystemHeader(Field));
   }
 }
diff --git a/clang/include/clang/ExtractAPI/Serialization/SerializerBase.h b/clang/include/clang/ExtractAPI/Serialization/SerializerBase.h
@@ -65,7 +65,7 @@ template <typename Derived> class APISetVisitor {
 
     getDerived()->traverseGlobalFunctionTemplateSpecializationRecords();
 
-    getDerived()->traverseStructRecords();
+    getDerived()->traverseRecordRecords();
 
     getDerived()->traverseObjCInterfaces();
 
@@ -98,9 +98,9 @@ template <typename Derived> class APISetVisitor {
       getDerived()->visitEnumRecord(*Enum.second);
   }
 
-  void traverseStructRecords() {
-    for (const auto &Struct : API.getStructs())
-      getDerived()->visitStructRecord(*Struct.second);
+  void traverseRecordRecords() {
+    for (const auto &Record : API.getRecords())
+      getDerived()->visitRecordRecord(*Record.second);
   }
 
   void traverseStaticFieldRecords() {
@@ -238,8 +238,8 @@ template <typename Derived> class APISetVisitor {
   /// Visit an enum record.
   void visitEnumRecord(const EnumRecord &Record){};
 
-  /// Visit a struct record.
-  void visitStructRecord(const StructRecord &Record){};
+  /// Visit a record record.
+  void visitRecordRecord(const RecordRecord &Record){};
 
   void visitStaticFieldRecord(const StaticFieldRecord &Record){};
 
diff --git a/clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h b/clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
@@ -170,8 +170,8 @@ class SymbolGraphSerializer : public APISetVisitor<SymbolGraphSerializer> {
   /// Visit an enum record.
   void visitEnumRecord(const EnumRecord &Record);
 
-  /// Visit a struct record.
-  void visitStructRecord(const StructRecord &Record);
+  /// Visit a record record.
+  void visitRecordRecord(const RecordRecord &Record);
 
   void visitStaticFieldRecord(const StaticFieldRecord &Record);
 
diff --git a/clang/lib/ExtractAPI/API.cpp b/clang/lib/ExtractAPI/API.cpp
@@ -145,31 +145,30 @@ EnumRecord *APISet::addEnum(StringRef Name, StringRef USR, PresumedLoc Loc,
                            SubHeading, IsFromSystemHeader);
 }
 
-StructFieldRecord *APISet::addStructField(StructRecord *Struct, StringRef Name,
-                                          StringRef USR, PresumedLoc Loc,
-                                          AvailabilitySet Availabilities,
-                                          const DocComment &Comment,
-                                          DeclarationFragments Declaration,
-                                          DeclarationFragments SubHeading,
-                                          bool IsFromSystemHeader) {
-  auto Record = std::make_unique<StructFieldRecord>(
+RecordFieldRecord *APISet::addRecordField(
+    RecordRecord *Record, StringRef Name, StringRef USR, PresumedLoc Loc,
+    AvailabilitySet Availabilities, const DocComment &Comment,
+    DeclarationFragments Declaration, DeclarationFragments SubHeading,
+    APIRecord::RecordKind Kind, bool IsFromSystemHeader) {
+  auto RecordField = std::make_unique<RecordFieldRecord>(
       USR, Name, Loc, std::move(Availabilities), Comment, Declaration,
-      SubHeading, IsFromSystemHeader);
-  Record->ParentInformation = APIRecord::HierarchyInformation(
-      Struct->USR, Struct->Name, Struct->getKind(), Struct);
-  USRBasedLookupTable.insert({USR, Record.get()});
-  return Struct->Fields.emplace_back(std::move(Record)).get();
+      SubHeading, Kind, IsFromSystemHeader);
+  RecordField->ParentInformation = APIRecord::HierarchyInformation(
+      Record->USR, Record->Name, Record->getKind(), Record);
+  USRBasedLookupTable.insert({USR, RecordField.get()});
+  return Record->Fields.emplace_back(std::move(RecordField)).get();
 }
 
-StructRecord *APISet::addStruct(StringRef Name, StringRef USR, PresumedLoc Loc,
+RecordRecord *APISet::addRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
                                 AvailabilitySet Availabilities,
                                 const DocComment &Comment,
                                 DeclarationFragments Declaration,
                                 DeclarationFragments SubHeading,
+                                APIRecord::RecordKind Kind,
                                 bool IsFromSystemHeader) {
-  return addTopLevelRecord(USRBasedLookupTable, Structs, USR, Name, Loc,
+  return addTopLevelRecord(USRBasedLookupTable, Records, USR, Name, Loc,
                            std::move(Availabilities), Comment, Declaration,
-                           SubHeading, IsFromSystemHeader);
+                           SubHeading, Kind, IsFromSystemHeader);
 }
 
 StaticFieldRecord *
@@ -548,8 +547,8 @@ void GlobalFunctionRecord::anchor() {}
 void GlobalVariableRecord::anchor() {}
 void EnumConstantRecord::anchor() {}
 void EnumRecord::anchor() {}
-void StructFieldRecord::anchor() {}
-void StructRecord::anchor() {}
+void RecordFieldRecord::anchor() {}
+void RecordRecord::anchor() {}
 void CXXFieldRecord::anchor() {}
 void CXXClassRecord::anchor() {}
 void CXXConstructorRecord::anchor() {}
diff --git a/clang/lib/ExtractAPI/DeclarationFragments.cpp b/clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -760,13 +760,16 @@ DeclarationFragmentsBuilder::getFragmentsForField(const FieldDecl *Field) {
       .append(";", DeclarationFragments::FragmentKind::Text);
 }
 
-DeclarationFragments
-DeclarationFragmentsBuilder::getFragmentsForStruct(const RecordDecl *Record) {
+DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForRecordDecl(
+    const RecordDecl *Record) {
   if (const auto *TypedefNameDecl = Record->getTypedefNameForAnonDecl())
     return getFragmentsForTypedef(TypedefNameDecl);
 
   DeclarationFragments Fragments;
-  Fragments.append("struct", DeclarationFragments::FragmentKind::Keyword);
+  if (Record->isUnion())
+    Fragments.append("union", DeclarationFragments::FragmentKind::Keyword);
+  else
+    Fragments.append("struct", DeclarationFragments::FragmentKind::Keyword);
 
   if (!Record->getName().empty())
     Fragments.appendSpace().append(
diff --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
diff --git a/clang/test/ExtractAPI/union.c b/clang/test/ExtractAPI/union.c
