[clang][extract-api] Add enum support

[zixuw]

Summary

Add support for enum records

• Add EnumConstantRecord and EnumRecord to store API information for enums
• Implement VisitEnumDecl in ExtractAPIVisitor
• Implement serializatin for enum records and MemberOf relationship
• Add test case for enum records
• Few other improvements

[dang]

clang/lib/SymbolGraph/ExtractAPIConsumer.cpp

197 ↗

(On Diff #416037)

Should this be static or in an anonymous namespace?

clang/lib/SymbolGraph/Serialization.cpp

228 ↗

(On Diff #416037)

Nice!

253 ↗

(On Diff #416037)

"Enumeration Case"

257 ↗

(On Diff #416037)

"Enumeration"

321 ↗

(On Diff #416037)

Since we are going to add more cases to this switch, would you mind doing something along the lines of:

const char *KindString = "";
switch(Kind) {
  default:
     llvm_unreachable("Unhandled relationship kind in Symbol Graph!");
     break;
  case RelationshipKind::MemberOf: 
    KindString = "memberOf";
    break;
}

Relationship["kind"] = KindString

or a method in the serializer for getting the string representation of the relationship kind?

343 ↗

(On Diff #416037)

Quick design question: Do we want to be silently failing in these situations (especially since this shouldn't be happening)? Let me know what you think.

[zixuw]

clang/lib/SymbolGraph/Serialization.cpp

321 ↗

(On Diff #416037)

Definitely! That's a good idea. I'm going to have a 'getString' sort of method in the serializer to do this.

343 ↗

(On Diff #416037)

I'm using this check to intentionally skip symbols that we do not want to spit out, for example unconditionally unavailable symbols, or after we have support for typedef records, anonymous enum decls that's declared with a typedef so that we don't have duplicate information, etc.
Optional<Object> serializeAPIRecord does this check now, and if we Serializer::shouldSkip it, None will be returned. So it is expected, not really silently failing.

[sammccall]

Hey, apologies for missing the initial patch in D119479.

This sounds really interesting!
The lack of docs make it very difficult to understand what this library is for and how to use it if you don't already know.
It's not clear what the SymbolGraph library is (I think there's a doc missing from the directory). The patches are tagged with [extract-api] so it must be related, but if it's something distinct it should be documented and reusable, and if it's the same why is it not called ExtractAPI :-)

The main entry point seems to be API.h, which has only a trivial file comment and no comments on what the meaning of the structs and fields are. Especially if this is part of clang proper, it's going to be read and maintained and debugged by a lot of people who have never heard of SymbolGraph and don't know swift.

It's also not clear from looking at this why this is part of clang itself and not a libTooling-style external tool - it seems similar in scope and applicability to a lot of the things in clang-tools-extra.

I know a lot of these things don't feel like they need much explanation when you're and your reviewer are familiar with it, so apologies that this is tedious, but the cost of undocumented code like this in shared projects is high.

[dang]

clang/lib/SymbolGraph/Serialization.cpp

343 ↗

(On Diff #416037)

Yeah that makes sense. If we are doing the check in serializeAPIRecord it might be worth not calling serializeEnumRecord at all for the sake of keeping the code handling these kinds of situation in a single place.

[zixuw]

Hi Sam! Thanks for your interest and taking a look!

In D121873#3391059, @sammccall wrote:

Hey, apologies for missing the initial patch in D119479.

This sounds really interesting!
The lack of docs make it very difficult to understand what this library is for and how to use it if you don't already know.
It's not clear what the SymbolGraph library is (I think there's a doc missing from the directory). The patches are tagged with [extract-api] so it must be related, but if it's something distinct it should be documented and reusable, and if it's the same why is it not called ExtractAPI :-)

My apologies for the confusion caused! Yes I do need to improve the docs for existing changes and put more efforts into the readability of the code in future patches. And I definitely agree that SymbolGraph is not really an appropriate name for the library. Initially the code was directly put in clangFrontend but I had to move them out to resolve a circular dependency problem between frontend and clangIndex. And it seems that I'm just bad at naming things 🙂.
I've put up another patch D122160 to refactor all the ExtractAPI work and also add missing doc comments to make things clearer.

The main entry point seems to be API.h, which has only a trivial file comment and no comments on what the meaning of the structs and fields are. Especially if this is part of clang proper, it's going to be read and maintained and debugged by a lot of people who have never heard of SymbolGraph and don't know swift.

It's also not clear from looking at this why this is part of clang itself and not a libTooling-style external tool - it seems similar in scope and applicability to a lot of the things in clang-tools-extra.

Implementing this with libTooling in clang-tools-extra is definitely something we've considered and evaluated during designing and before the proposal. And we decided that including this in clang core had great benefits:

• Implementing ExtractAPI in clang itself allows the functionalities to be easily accessible by more downstream tools to use either the API information collected or the serialized output via clang or libclang, for example IDE integrations.
• We could share parts of the code with Swift down the line.
• The proposed clang-extract-api tool only need to parse a set of given header files for flexibility and efficiency. Implementing ExtractAPI as a clang driver allows a more flexible control of invoking the tool with various of clang options without the need of a compilation database, and be decoupled from compilations and builds.

I know a lot of these things don't feel like they need much explanation when you're and your reviewer are familiar with it, so apologies that this is tedious, but the cost of undocumented code like this in shared projects is high.

Not at all! There are all valid and great feedbacks and questions. Thank you very much for pointing there out Sam! I've definitely neglected documenting and presenting the code for better readability and community collaboration. It's always welcome and exciting to have these feedbacks to move the tool forward together.

[zixuw]

Rebase onto D122160 refactoring

[zixuw]

• Address review issues;
• Fix memory managment for EnumConstantRecord and EnumRecord

clang/lib/SymbolGraph/ExtractAPIConsumer.cpp

197 ↗

(On Diff #416037)

The whole ExtractAPIVisitor class is already in an anonymous namespace right?

[dang]

clang/include/clang/ExtractAPI/API.h

33

I still think this belongs in APISet. It belongs there because it defines unique pointers that are specifically tied to the lifetime of APISet. Any other thing that needs a reference to things allocated in the APISet should just get the raw pointer out, ideally they should be storing the pointer returned by the add* methods of APISet.

[zixuw]

Rebase on main with changed and merged refactoring in D122160

[dang]

LGTM!

[mstorsjo]

This (or some closely related commit?) causes a huge amount of warnings when building with GCC:

warning: ‘clang::extractapi::EnumRecord’ has a field ‘clang::extractapi::EnumRecord::Constants’ whose type uses the anonymous namespace [-Wsubobject-linkage]
 167 | struct EnumRecord : APIRecord {
     |        ^~~~~~~~~~

[zixuw]

In D121873#3403082, @mstorsjo wrote:

This (or some closely related commit?) causes a huge amount of warnings when building with GCC:

warning: ‘clang::extractapi::EnumRecord’ has a field ‘clang::extractapi::EnumRecord::Constants’ whose type uses the anonymous namespace [-Wsubobject-linkage]
 167 | struct EnumRecord : APIRecord {
     |        ^~~~~~~~~~

Hi Martin! Thanks for the update on the warnings on GCC, didn't know about that.
I think the warning is complaining about the use of APIRecordUniquePtr, which should be removed by D122331.

[dexonsmith]

clang/test/ExtractAPI/enum.c

5

I just noticed these tests use %clang instead of %clang_cc1. Usually it's best to test the driver in clang/test/Driver using -### (does the driver produce the right -cc1 flags?) and then the compilation in isolation here using %clang_cc1 and a -cc1 command-line. Is it possible to update the tests to do that?

6

I think this will *always* be empty, even if there are warnings or errors, since those get written to stderr (not stdout). You could fix that by putting 2>&1 before the pipe, but see below:

13–14

It's usually much easier to test diagnostics using clangs builtin -verify flag instead of FileCheck. -verify is documented in the header: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Frontend/VerifyDiagnosticConsumer.h (maybe elsewhere too?).

This gives you good diffs between expected and unexpected diagnostics. In this case, you could replace these CHECK-NOTs with:

// expected-no-diagnostics

Usually you'd put that after the RUN line. When diagnostics are expected, you usually put those comments on the same line where you expect the diagnostic (there are tricks to avoid long lines and to handle other locations; see the docs).

[dexonsmith]

clang/test/ExtractAPI/enum.c

11

Diffing against reference output is usually not ideal:

• It makes it hard to evolve the output, since many unrelated tests could be affected and need an update. In practice, people may just regenerate the output without really knowing if the essential feature is still behaving correctly.
• It makes it hard to evolve the tests, since line numbers are hardcoded.

Is it possible to update to use FileCheck for the output? You can just check the parts of the output that are relevant for this specific test, embed CHECKs directly into the input file, and use @LINE so that the line numbers tied to the CHECK comments instead of hardcoded.

13–14

As an example I just posted https://reviews.llvm.org/D124634 for this file; PTAL, and then it'd be great if you could update the other tests when you get a chance.

136

(I noticed the diff when adding // expected-no-diagnostics as part of https://reviews.llvm.org/D124634 -- I couldn't add it to the top of the input file, since that disrupted all of the line numbers and caused the diff to fail. An easy workaround in this case was just to move it to the end of the file so I wasn't blocked but it'd be better if the test were able to evolve over time I think.)

clang/include/clang/ExtractAPI/API.h

 #include "clang/ExtractAPI/DeclarationFragments.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include <memory>
 
+namespace {
+
+/// \brief A custom deleter used for ``std::unique_ptr`` to APIRecords stored
+/// in the BumpPtrAllocator.
+///
+/// \tparam T the exact type of the APIRecord subclass.
+template <typename T> struct UniquePtrBumpPtrAllocatorDeleter {
+  void operator()(T *Instance) { Instance->~T(); }
+};
+
+/// A unique pointer to an APIRecord stored in the BumpPtrAllocator.
+///
+/// \tparam T the exact type of the APIRecord subclass.
+template <typename T>
+using APIRecordUniquePtr =
+    std::unique_ptr<T, UniquePtrBumpPtrAllocatorDeleter<T>>;
+
+} // anonymous namespace
+
 namespace clang {
 namespace extractapi {
 
 /// DocComment is a vector of RawComment::CommentLine.
 ///
 /// Each line represents one line of striped documentation comment,
 /// with source range information. This simplifies calculating the source
 /// location of a character in the doc comment for pointing back to the source
struct APIRecord {
   /// SubHeading is an array of declaration fragments of tagged declaration
   /// name, with potentially more tokens (for example the \c +/- symbol for
   /// Objective-C class/instance methods).
   DeclarationFragments SubHeading;
 
   /// Discriminator for LLVM-style RTTI (dyn_cast<> et al.)
   enum RecordKind {
     RK_Global,
+    RK_EnumConstant,
+    RK_Enum,
   };
 
 private:
   const RecordKind Kind;
 
 public:
   RecordKind getKind() const { return Kind; }
 
struct GlobalRecord : APIRecord {
   static bool classof(const APIRecord *Record) {
     return Record->getKind() == RK_Global;
   }
 
 private:
   virtual void anchor();
 };
 
+/// This holds information associated with enum constants.
+struct EnumConstantRecord : APIRecord {
+  EnumConstantRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
+                     const AvailabilityInfo &Availability,
+                     const DocComment &Comment,
+                     DeclarationFragments Declaration,
+                     DeclarationFragments SubHeading)
+      : APIRecord(RK_EnumConstant, Name, USR, Loc, Availability,
+                  LinkageInfo::none(), Comment, Declaration, SubHeading) {}
+
+  static bool classof(const APIRecord *Record) {
+    return Record->getKind() == RK_EnumConstant;
+  }
+};
+
+/// This holds information associated with enums.
+struct EnumRecord : APIRecord {
+  SmallVector<APIRecordUniquePtr<EnumConstantRecord>> Constants;
+
+  EnumRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
+             const AvailabilityInfo &Availability, const DocComment &Comment,
+             DeclarationFragments Declaration, DeclarationFragments SubHeading)
+      : APIRecord(RK_Enum, Name, USR, Loc, Availability, LinkageInfo::none(),
+                  Comment, Declaration, SubHeading) {}
+
+  static bool classof(const APIRecord *Record) {
+    return Record->getKind() == RK_Enum;
+  }
+};
+
 /// APISet holds the set of API records collected from given inputs.
 class APISet {
 public:
   /// Create and add a GlobalRecord of kind \p Kind 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
public:
   /// to generate the USR for \c D and keep it alive in APISet.
   GlobalRecord *addFunction(StringRef Name, StringRef USR, PresumedLoc Loc,
                             const AvailabilityInfo &Availability,
                             LinkageInfo Linkage, const DocComment &Comment,
                             DeclarationFragments Declaration,
                             DeclarationFragments SubHeading,
                             FunctionSignature Signature);
 
-private:
-  /// \brief A custom deleter used for ``std::unique_ptr`` to APIRecords stored
-  /// in the BumpPtrAllocator.
+  /// Create and add an enum constant record into the API set.
   ///
-  /// \tparam T the exact type of the APIRecord subclass.
-  template <typename T> struct UniquePtrBumpPtrAllocatorDeleter {
-    void operator()(T *Instance) { Instance->~T(); }
-  };
+  /// 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.
+  EnumConstantRecord *addEnumConstant(EnumRecord *Enum, StringRef Name,
+                                      StringRef USR, PresumedLoc Loc,
+                                      const AvailabilityInfo &Availability,
+                                      const DocComment &Comment,
+                                      DeclarationFragments Declaration,
+                                      DeclarationFragments SubHeading);
 
-public:
-  /// A unique pointer to an APIRecord stored in the BumpPtrAllocator.
+  /// Create and add an enum record into the API set.
   ///
-  /// \tparam T the exact type of the APIRecord subclass.
-  template <typename T>
-  using APIRecordUniquePtr =
-      std::unique_ptr<T, UniquePtrBumpPtrAllocatorDeleter<T>>;
+  /// 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.
+  EnumRecord *addEnum(StringRef Name, StringRef USR, PresumedLoc Loc,
+                      const AvailabilityInfo &Availability,
+                      const DocComment &Comment,
+                      DeclarationFragments Declaration,
+                      DeclarationFragments SubHeading);
 
   /// A map to store the set of GlobalRecord%s with the declaration name as the
   /// key.
   using GlobalRecordMap =
       llvm::MapVector<StringRef, APIRecordUniquePtr<GlobalRecord>>;
 
+  /// A map to store the set of EnumRecord%s with the declaration name as the
+  /// key.
+  using EnumRecordMap =
+      llvm::MapVector<StringRef, APIRecordUniquePtr<EnumRecord>>;
+
   /// Get the target triple for the ExtractAPI invocation.
   const llvm::Triple &getTarget() const { return Target; }
 
   /// Get the language options used to parse the APIs.
   const LangOptions &getLangOpts() const { return LangOpts; }
 
   const GlobalRecordMap &getGlobals() const { return Globals; }
+  const EnumRecordMap &getEnums() const { return Enums; }
 
   /// Generate and store the USR of declaration \p D.
   ///
   /// Note: The USR string is stored in and owned by Allocator.
   ///
   /// \returns a StringRef of the generated USR string.
   StringRef recordUSR(const Decl *D);
 
   /// Copy \p String into the Allocator in this APISet.
   ///
   /// \returns a StringRef of the copied string in APISet::Allocator.
   StringRef copyString(StringRef String);
 
   APISet(const llvm::Triple &Target, const LangOptions &LangOpts)
       : Target(Target), LangOpts(LangOpts) {}
 
 private:
   /// BumpPtrAllocator to store APIRecord%s and generated/copied strings.
   llvm::BumpPtrAllocator Allocator;
 
   const llvm::Triple Target;
   const LangOptions LangOpts;
 
   GlobalRecordMap Globals;
+  EnumRecordMap Enums;
 };
 
 } // namespace extractapi
 } // namespace clang
 
 #endif // LLVM_CLANG_EXTRACTAPI_API_H

clang/include/clang/ExtractAPI/DeclarationFragments.h

 class DeclarationFragmentsBuilder {
 public:
   /// Build DeclarationFragments for a variable declaration VarDecl.
   static DeclarationFragments getFragmentsForVar(const VarDecl *);
 
   /// Build DeclarationFragments for a function declaration FunctionDecl.
   static DeclarationFragments getFragmentsForFunction(const FunctionDecl *);
 
+  /// Build DeclarationFragments for an enum constant declaration
+  /// EnumConstantDecl.
+  static DeclarationFragments
+  getFragmentsForEnumConstant(const EnumConstantDecl *);
+
+  /// Build DeclarationFragments for an enum declaration EnumDecl.
+  static DeclarationFragments getFragmentsForEnum(const EnumDecl *);
+
   /// Build sub-heading fragments for a NamedDecl.
   static DeclarationFragments getSubHeading(const NamedDecl *);
 
   /// Build FunctionSignature for a function declaration FunctionDecl.
   static FunctionSignature getFunctionSignature(const FunctionDecl *);
 
 private:
   DeclarationFragmentsBuilder() = delete;

clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h

public:
   /// \returns a JSON object that contains the root of the formatted
   /// Symbol Graph.
   Object serialize();
 
   /// Implement the APISerializer::serialize interface. Wrap serialize(void) and
   /// write out the serialized JSON object to \p os.
   void serialize(raw_ostream &os) override;
 
+  /// The kind of a relationship between two symbols.
+  enum RelationshipKind {
+    /// The source symbol is a member of the target symbol.
+    /// For example enum constants are members of the enum, class/instance
+    /// methods are members of the class, etc.
+    MemberOf,
+  };
+
+  /// Get the string representation of the relationship kind.
+  static StringRef getRelationshipString(RelationshipKind Kind);
+
 private:
   /// Synthesize the metadata section of the Symbol Graph format.
   ///
   /// The metadata section describes information about the Symbol Graph itself,
   /// including the format version and the generator information.
   Object serializeMetadata() const;
 
   /// Synthesize the module section of the Symbol Graph format.
private:
   /// augmented with kind-specific symbol information by the caller.
   /// This method also checks if the given \p Record should be skipped during
   /// serialization.
   ///
   /// \returns \c None if this \p Record should be skipped, or a JSON object
   /// containing common symbol information of \p Record.
   Optional<Object> serializeAPIRecord(const APIRecord &Record) const;
 
+  /// Serialize the \p Kind relationship between \p Source and \p Target.
+  ///
+  /// Record the relationship between the two symbols in
+  /// SymbolGraphSerializer::Relationships.
+  void serializeRelationship(RelationshipKind Kind, const APIRecord &Source,
+                             const APIRecord &Target);
+
   /// Serialize a global record.
   void serializeGlobalRecord(const GlobalRecord &Record);
 
+  /// Serialize an enum record.
+  void serializeEnumRecord(const EnumRecord &Record);
+
 public:
   SymbolGraphSerializer(const APISet &API, StringRef ProductName,
                         APISerializerOption Options = {})
       : APISerializer(API, ProductName, Options) {}
 };
 
 } // namespace extractapi
 } // namespace clang
 
 #endif // LLVM_CLANG_EXTRACTAPI_SERIALIZATION_SYMBOLGRAPHSERIALIZER_H

clang/lib/ExtractAPI/API.cpp

APISet::addFunction(StringRef Name, StringRef USR, PresumedLoc Loc,
                     const AvailabilityInfo &Availability, LinkageInfo Linkage,
                     const DocComment &Comment, DeclarationFragments Fragments,
                     DeclarationFragments SubHeading,
                     FunctionSignature Signature) {
   return addGlobal(GVKind::Function, Name, USR, Loc, Availability, Linkage,
                    Comment, Fragments, SubHeading, Signature);
 }
 
+EnumConstantRecord *APISet::addEnumConstant(
+    EnumRecord *Enum, StringRef Name, StringRef USR, PresumedLoc Loc,
+    const AvailabilityInfo &Availability, const DocComment &Comment,
+    DeclarationFragments Declaration, DeclarationFragments SubHeading) {
+  auto Record =
+      APIRecordUniquePtr<EnumConstantRecord>(new (Allocator) EnumConstantRecord{
+          Name, USR, Loc, Availability, Comment, Declaration, SubHeading});
+  return Enum->Constants.emplace_back(std::move(Record)).get();
+}
+
+EnumRecord *APISet::addEnum(StringRef Name, StringRef USR, PresumedLoc Loc,
+                            const AvailabilityInfo &Availability,
+                            const DocComment &Comment,
+                            DeclarationFragments Declaration,
+                            DeclarationFragments SubHeading) {
+  auto Result = Enums.insert({Name, nullptr});
+  if (Result.second) {
+    // Create the record if it does not already exist.
+    auto Record = APIRecordUniquePtr<EnumRecord>(new (Allocator) EnumRecord{
+        Name, USR, Loc, Availability, Comment, Declaration, SubHeading});
+    Result.first->second = std::move(Record);
+  }
+  return Result.first->second.get();
+}
+
 StringRef APISet::recordUSR(const Decl *D) {
   SmallString<128> USR;
   index::generateUSRForDecl(D, USR);
   return copyString(USR);
 }
 
 StringRef APISet::copyString(StringRef String) {
   if (String.empty())

clang/lib/ExtractAPI/DeclarationFragments.cpp

for (unsigned i = 0, end = Func->getNumParams(); i != end; ++i) {
     Fragments.append(getFragmentsForParam(Func->getParamDecl(i)));
   }
   Fragments.append(")", DeclarationFragments::FragmentKind::Text);
 
   // FIXME: Handle exception specifiers: throw, noexcept
   return Fragments;
 }
 
+DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForEnumConstant(
+    const EnumConstantDecl *EnumConstDecl) {
+  DeclarationFragments Fragments;
+  return Fragments.append(EnumConstDecl->getName(),
+                          DeclarationFragments::FragmentKind::Identifier);
+}
+
+DeclarationFragments
+DeclarationFragmentsBuilder::getFragmentsForEnum(const EnumDecl *EnumDecl) {
+  // TODO: After we support typedef records, if there's a typedef for this enum
+  // just use the declaration fragments of the typedef decl.
+
+  DeclarationFragments Fragments, After;
+  Fragments.append("enum", DeclarationFragments::FragmentKind::Keyword);
+
+  if (!EnumDecl->getName().empty())
+    Fragments.appendSpace().append(
+        EnumDecl->getName(), DeclarationFragments::FragmentKind::Identifier);
+
+  QualType IntegerType = EnumDecl->getIntegerType();
+  if (!IntegerType.isNull())
+    Fragments.append(": ", DeclarationFragments::FragmentKind::Text)
+        .append(
+            getFragmentsForType(IntegerType, EnumDecl->getASTContext(), After))
+        .append(std::move(After));
+
+  return Fragments;
+}
+
 FunctionSignature
 DeclarationFragmentsBuilder::getFunctionSignature(const FunctionDecl *Func) {
   FunctionSignature Signature;
 
   for (const auto *Param : Func->parameters()) {
     StringRef Name = Param->getName();
     DeclarationFragments Fragments = getFragmentsForParam(Param);
 

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

FunctionSignature Signature =
         DeclarationFragmentsBuilder::getFunctionSignature(Decl);
 
     // Add the function record to the API set.
     API.addFunction(Name, USR, Loc, Availability, Linkage, Comment, Declaration,
                     SubHeading, Signature);
     return true;
   }
 
+  bool VisitEnumDecl(const EnumDecl *Decl) {
+    if (!Decl->isComplete())
+      return true;
+
+    // Skip forward declaration.
+    if (!Decl->isThisDeclarationADefinition())
+      return true;
+
+    // Collect symbol information.
+    StringRef Name = Decl->getName();
+    StringRef USR = API.recordUSR(Decl);
+    PresumedLoc Loc =
+        Context.getSourceManager().getPresumedLoc(Decl->getLocation());
+    AvailabilityInfo Availability = getAvailability(Decl);
+    DocComment Comment;
+    if (auto *RawComment = Context.getRawCommentForDeclNoCache(Decl))
+      Comment = RawComment->getFormattedLines(Context.getSourceManager(),
+                                              Context.getDiagnostics());
+
+    // Build declaration fragments and sub-heading for the enum.
+    DeclarationFragments Declaration =
+        DeclarationFragmentsBuilder::getFragmentsForEnum(Decl);
+    DeclarationFragments SubHeading =
+        DeclarationFragmentsBuilder::getSubHeading(Decl);
+
+    EnumRecord *EnumRecord = API.addEnum(Name, USR, Loc, Availability, Comment,
+                                         Declaration, SubHeading);
+
+    // Now collect information about the enumerators in this enum.
+    recordEnumConstants(EnumRecord, Decl->enumerators());
+
+    return true;
+  }
+
 private:
   /// Get availability information of the declaration \p D.
   AvailabilityInfo getAvailability(const Decl *D) const {
     StringRef PlatformName = Context.getTargetInfo().getPlatformName();
 
     AvailabilityInfo Availability;
     // Collect availability attributes from all redeclarations.
     for (const auto *RD : D->redecls()) {
for (const auto *RD : D->redecls()) {
       if (const auto *A = RD->getAttr<DeprecatedAttr>())
         if (!A->isImplicit())
           Availability.UnconditionallyDeprecated = true;
     }
 
     return Availability;
   }
 
+  /// Collect API information for the enum constants and associate with the
+  /// parent enum.
+  void recordEnumConstants(EnumRecord *EnumRecord,
+                           const EnumDecl::enumerator_range Constants) {
+    for (const auto *Constant : Constants) {
+      // Collect symbol information.
+      StringRef Name = Constant->getName();
+      StringRef USR = API.recordUSR(Constant);
+      PresumedLoc Loc =
+          Context.getSourceManager().getPresumedLoc(Constant->getLocation());
+      AvailabilityInfo Availability = getAvailability(Constant);
+      DocComment Comment;
+      if (auto *RawComment = Context.getRawCommentForDeclNoCache(Constant))
+        Comment = RawComment->getFormattedLines(Context.getSourceManager(),
+                                                Context.getDiagnostics());
+
+      // Build declaration fragments and sub-heading for the enum constant.
+      DeclarationFragments Declaration =
+          DeclarationFragmentsBuilder::getFragmentsForEnumConstant(Constant);
+      DeclarationFragments SubHeading =
+          DeclarationFragmentsBuilder::getSubHeading(Constant);
+
+      API.addEnumConstant(EnumRecord, Name, USR, Loc, Availability, Comment,
+                          Declaration, SubHeading);
+    }
+  }
+
   ASTContext &Context;
   APISet API;
 };
 
 class ExtractAPIConsumer : public ASTConsumer {
 public:
   ExtractAPIConsumer(ASTContext &Context, StringRef ProductName,
                      std::unique_ptr<raw_pwrite_stream> OS)

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

 
 /// Serialize the symbol kind information.
 ///
 /// The Symbol Graph symbol kind property contains a shorthand \c identifier
 /// which is prefixed by the source language name, useful for tooling to parse
 /// the kind, and a \c displayName for rendering human-readable names.
 Object serializeSymbolKind(const APIRecord &Record,
                            const LangOptions &LangOpts) {
+  auto AddLangPrefix = [&LangOpts](StringRef S) -> std::string {
+    return (getLanguageName(LangOpts) + "." + S).str();
+  };
+
   Object Kind;
   switch (Record.getKind()) {
-  case APIRecord::RK_Global:
+  case APIRecord::RK_Global: {
     auto *GR = dyn_cast<GlobalRecord>(&Record);
     switch (GR->GlobalKind) {
     case GVKind::Function:
-      Kind["identifier"] = (getLanguageName(LangOpts) + ".func").str();
+      Kind["identifier"] = AddLangPrefix("func");
       Kind["displayName"] = "Function";
       break;
     case GVKind::Variable:
-      Kind["identifier"] = (getLanguageName(LangOpts) + ".var").str();
+      Kind["identifier"] = AddLangPrefix("var");
       Kind["displayName"] = "Global Variable";
       break;
     case GVKind::Unknown:
       // Unknown global kind
       break;
     }
     break;
   }
+  case APIRecord::RK_EnumConstant:
+    Kind["identifier"] = AddLangPrefix("enum.case");
+    Kind["displayName"] = "Enumeration Case";
+    break;
+  case APIRecord::RK_Enum:
+    Kind["identifier"] = AddLangPrefix("enum");
+    Kind["displayName"] = "Enumeration";
+    break;
+  }
 
   return Kind;
 }
 
 } // namespace
 
 void SymbolGraphSerializer::anchor() {}
 
serializeObject(Obj, "availbility",
                   serializeAvailability(Record.Availability));
   serializeObject(Obj, "docComment", serializeDocComment(Record.Comment));
   serializeArray(Obj, "declarationFragments",
                  serializeDeclarationFragments(Record.Declaration));
 
   return Obj;
 }
 
+StringRef SymbolGraphSerializer::getRelationshipString(RelationshipKind Kind) {
+  switch (Kind) {
+  case RelationshipKind::MemberOf:
+    return "memberOf";
+  }
+  llvm_unreachable("Unhandled relationship kind");
+}
+
+void SymbolGraphSerializer::serializeRelationship(RelationshipKind Kind,
+                                                  const APIRecord &Source,
+                                                  const APIRecord &Target) {
+  Object Relationship;
+  Relationship["source"] = Source.USR;
+  Relationship["target"] = Target.USR;
+  Relationship["kind"] = getRelationshipString(Kind);
+
+  Relationships.emplace_back(std::move(Relationship));
+}
+
 void SymbolGraphSerializer::serializeGlobalRecord(const GlobalRecord &Record) {
   auto Obj = serializeAPIRecord(Record);
   if (!Obj)
     return;
 
   if (Record.GlobalKind == GVKind::Function)
     serializeObject(*Obj, "parameters",
                     serializeFunctionSignature(Record.Signature));
 
   Symbols.emplace_back(std::move(*Obj));
 }
 
+void SymbolGraphSerializer::serializeEnumRecord(const EnumRecord &Record) {
+  auto Enum = serializeAPIRecord(Record);
+  if (!Enum)
+    return;
+
+  Symbols.emplace_back(std::move(*Enum));
+
+  for (const auto &Constant : Record.Constants) {
+    auto EnumConstant = serializeAPIRecord(*Constant);
+    if (!EnumConstant)
+      continue;
+
+    Symbols.emplace_back(std::move(*EnumConstant));
+    serializeRelationship(RelationshipKind::MemberOf, *Constant, Record);
+  }
+}
+
 Object SymbolGraphSerializer::serialize() {
   Object Root;
   serializeObject(Root, "metadata", serializeMetadata());
   serializeObject(Root, "module", serializeModule());
 
   // Serialize global records in the API set.
   for (const auto &Global : API.getGlobals())
     serializeGlobalRecord(*Global.second);
 
+  // Serialize enum records in the API set.
+  for (const auto &Enum : API.getEnums())
+    serializeEnumRecord(*Enum.second);
+
   Root["symbols"] = std::move(Symbols);
   Root["relationhips"] = std::move(Relationships);
 
   return Root;
 }
 
 void SymbolGraphSerializer::serialize(raw_ostream &os) {
   Object root = serialize();
   if (Options.Compact)
     os << formatv("{0}", Value(std::move(root))) << "\n";
   else
     os << formatv("{0:2}", Value(std::move(root))) << "\n";
 }

clang/test/ExtractAPI/enum.c

+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %t/reference.output.json
+// RUN: %clang -extract-api -target arm64-apple-macosx \
+// RUN: %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- input.h
+/// Kinds of vehicles
+enum Vehicle {
+  Bicycle,
+  Car,
+  Train, ///< Move this to the top! -Sheldon
+  Ship,
+  Airplane,
+};
+
+enum Direction : unsigned char {
+  North = 0,
+  East,
+  South,
+  West
+};
+
+//--- reference.output.json.in
+{
+  "metadata": {
+    "formatVersion": {
+      "major": 0,
+      "minor": 5,
+      "patch": 3
+    },
+    "generator": "?"
+  },
+  "module": {
+    "name": "",
+    "platform": {
+      "architecture": "arm64",
+      "operatingSystem": {
+        "minimumVersion": {
+          "major": 11,
+          "minor": 0,
+          "patch": 0
+        },
+        "name": "macosx"
+      },
+      "vendor": "apple"
+    }
+  },
+  "relationhips": [
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Vehicle@Bicycle",
+      "target": "c:@E@Vehicle"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Vehicle@Car",
+      "target": "c:@E@Vehicle"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Vehicle@Train",
+      "target": "c:@E@Vehicle"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Vehicle@Ship",
+      "target": "c:@E@Vehicle"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Vehicle@Airplane",
+      "target": "c:@E@Vehicle"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Direction@North",
+      "target": "c:@E@Direction"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Direction@East",
+      "target": "c:@E@Direction"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Direction@South",
+      "target": "c:@E@Direction"
+    },
+    {
+      "kind": "memberOf",
+      "source": "c:@E@Direction@West",
+      "target": "c:@E@Direction"
+    }
+  ],
+  "symbols": [
+    {
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "enum"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "Vehicle"
+        },
+        {
+          "kind": "text",
+          "spelling": ": "
+        },
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:i",
+          "spelling": "unsigned int"
+        }
+      ],
+      "docComment": {
+        "lines": [
+          {
+            "range": {
+              "end": {
+                "character": 22,
+                "line": 1
+              },
+              "start": {
+                "character": 5,
+                "line": 1
+              }
+            },
+            "text": "Kinds of vehicles"
+          }
+        ]
+      },
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Vehicle"
+      },
+      "kind": {
+        "displayName": "Enumeration",
+        "identifier": "c.enum"
+      },
+      "location": {
+        "character": 6,
+        "line": 2,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Vehicle"
+          }
+        ],
+        "title": "Vehicle"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "Bicycle"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Vehicle@Bicycle"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 3,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Bicycle"
+          }
+        ],
+        "title": "Bicycle"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "Car"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Vehicle@Car"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 4,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Car"
+          }
+        ],
+        "title": "Car"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "Train"
+        }
+      ],
+      "docComment": {
+        "lines": [
+          {
+            "range": {
+              "end": {
+                "character": 45,
+                "line": 5
+              },
+              "start": {
+                "character": 15,
+                "line": 5
+              }
+            },
+            "text": "Move this to the top! -Sheldon"
+          }
+        ]
+      },
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Vehicle@Train"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 5,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Train"
+          }
+        ],
+        "title": "Train"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "Ship"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Vehicle@Ship"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 6,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Ship"
+          }
+        ],
+        "title": "Ship"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "Airplane"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Vehicle@Airplane"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 7,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Airplane"
+          }
+        ],
+        "title": "Airplane"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "keyword",
+          "spelling": "enum"
+        },
+        {
+          "kind": "text",
+          "spelling": " "
+        },
+        {
+          "kind": "identifier",
+          "spelling": "Direction"
+        },
+        {
+          "kind": "text",
+          "spelling": ": "
+        },
+        {
+          "kind": "typeIdentifier",
+          "preciseIdentifier": "c:c",
+          "spelling": "unsigned char"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Direction"
+      },
+      "kind": {
+        "displayName": "Enumeration",
+        "identifier": "c.enum"
+      },
+      "location": {
+        "character": 6,
+        "line": 10,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "Direction"
+          }
+        ],
+        "title": "Direction"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "North"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Direction@North"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 11,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "North"
+          }
+        ],
+        "title": "North"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "East"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Direction@East"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 12,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "East"
+          }
+        ],
+        "title": "East"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "South"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Direction@South"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 13,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "South"
+          }
+        ],
+        "title": "South"
+      }
+    },
+    {
+      "declarationFragments": [
+        {
+          "kind": "identifier",
+          "spelling": "West"
+        }
+      ],
+      "identifier": {
+        "interfaceLanguage": "c",
+        "precise": "c:@E@Direction@West"
+      },
+      "kind": {
+        "displayName": "Enumeration Case",
+        "identifier": "c.enum.case"
+      },
+      "location": {
+        "character": 3,
+        "line": 14,
+        "uri": "file://INPUT_DIR/input.h"
+      },
+      "names": {
+        "subHeading": [
+          {
+            "kind": "identifier",
+            "spelling": "West"
+          }
+        ],
+        "title": "West"
+      }
+    }
+  ]
+}