Address review feedback

Author: daniel-grumberg

clang/include/clang/ExtractAPI/API.h

@@ -195,7 +195,8 @@ struct APIRecord {
     RK_Unknown,
     // If adding a record context record kind here make sure to update
     // RecordContext::classof if needed and add a RECORD_CONTEXT entry to
-    // APIRecords.inc RecordContext Kinds start
+    // APIRecords.inc
+    RK_FirstRecordContext,
     RK_Namespace,
     RK_Enum,
     RK_Struct,
@@ -207,7 +208,7 @@ struct APIRecord {
     RK_ClassTemplate,
     RK_ClassTemplateSpecialization,
     RK_ClassTemplatePartialSpecialization,
-    // RecordContexts Kinds end
+    RK_LastRecordContext,
     RK_GlobalFunction,
     RK_GlobalFunctionTemplate,
     RK_GlobalFunctionTemplateSpecialization,
@@ -270,10 +271,10 @@ struct APIRecord {
   friend class RecordContext;
   // Used to store the next child record in RecordContext. This works because
   // APIRecords semantically only have one parent.
-  mutable APIRecord *NextInContex = nullptr;
+  mutable APIRecord *NextInContext = nullptr;
 
 public:
-  APIRecord *getNextInContex() const { return NextInContex; }
+  APIRecord *getNextInContext() const { return NextInContext; }
 
   RecordKind getKind() const { return Kind; }
 
@@ -312,8 +313,8 @@ class RecordContext {
     return classofKind(Record->getKind());
   }
   static bool classofKind(APIRecord::RecordKind K) {
-    return K >= APIRecord::RK_Namespace &&
-           K <= APIRecord::RK_ClassTemplatePartialSpecialization;
+    return K > APIRecord::RK_FirstRecordContext &&
+           K < APIRecord::RK_LastRecordContext;
   }
 
   static bool classof(const RecordContext *Context) { return true; }
@@ -339,7 +340,8 @@ class RecordContext {
     // This doesn't strictly meet the iterator requirements, but it's the
     // behavior we want here.
     value_type operator->() const { return Current; }
-    record_iterator &operator++() { Current = Current->getNextInContex();
+    record_iterator &operator++() {
+      Current = Current->getNextInContext();
       return *this;
     }
     record_iterator operator++(int) {
@@ -1421,8 +1423,8 @@ APISet::createRecord(StringRef USR, StringRef Name,
 
   // Create the record if it does not already exist
   if (Result.second) {
-    Record = new (Allocator) RecordTy(USRString, copyString(Name),
-                                std::forward<CtorArgsContTy>(CtorArgs)...);
+    Record = new (Allocator) RecordTy(
+        USRString, copyString(Name), std::forward<CtorArgsContTy>(CtorArgs)...);
     // Store the record in the record lookup map
     Result.first->second = APIRecordStoredPtr(Record);
 

clang/include/clang/ExtractAPI/ExtractAPIVisitor.h

@@ -631,8 +631,8 @@ bool ExtractAPIVisitorBase<Derived>::VisitCXXMethodDecl(
   auto Access = DeclarationFragmentsBuilder::getAccessControl(Decl);
   auto Signature = DeclarationFragmentsBuilder::getFunctionSignature(Decl);
 
-  if (Decl->isTemplated()) {
-    FunctionTemplateDecl *TemplateDecl = Decl->getDescribedFunctionTemplate();
+  if (FunctionTemplateDecl *TemplateDecl =
+          Decl->getDescribedFunctionTemplate()) {
     API.createRecord<CXXMethodTemplateRecord>(
         USR, Decl->getName(), createHierarchyInformationForDecl(*Decl), Loc,
         AvailabilityInfo::createFromDecl(Decl), Comment,
@@ -697,7 +697,7 @@ bool ExtractAPIVisitorBase<Derived>::VisitCXXConstructorDecl(
       DeclarationFragmentsBuilder::getFunctionSignature(Decl);
   AccessControl Access = DeclarationFragmentsBuilder::getAccessControl(Decl);
 
-  API.createRecord<CXXInstanceMethodRecord>(
+  API.createRecord<CXXConstructorRecord>(
       USR, Name, createHierarchyInformationForDecl(*Decl), Loc,
       AvailabilityInfo::createFromDecl(Decl), Comment, Declaration, SubHeading,
       Signature, Access, isInSystemHeader(Decl));
@@ -730,7 +730,7 @@ bool ExtractAPIVisitorBase<Derived>::VisitCXXDestructorDecl(
   FunctionSignature Signature =
       DeclarationFragmentsBuilder::getFunctionSignature(Decl);
   AccessControl Access = DeclarationFragmentsBuilder::getAccessControl(Decl);
-  API.createRecord<CXXInstanceMethodRecord>(
+  API.createRecord<CXXDestructorRecord>(
       USR, Name, createHierarchyInformationForDecl(*Decl), Loc,
       AvailabilityInfo::createFromDecl(Decl), Comment, Declaration, SubHeading,
       Signature, Access, isInSystemHeader(Decl));
@@ -1247,6 +1247,10 @@ bool ExtractAPIVisitorBase<Derived>::VisitFieldDecl(const FieldDecl *Decl) {
 template <typename Derived>
 bool ExtractAPIVisitorBase<Derived>::VisitCXXConversionDecl(
     const CXXConversionDecl *Decl) {
+  if (!getDerivedExtractAPIVisitor().shouldDeclBeIncluded(Decl) ||
+      Decl->isImplicit())
+    return true;
+
   auto Name = Decl->getNameAsString();
   SmallString<128> USR;
   index::generateUSRForDecl(Decl, USR);

clang/include/clang/ExtractAPI/Serialization/APISetVisitor.h

@@ -36,7 +36,7 @@ namespace extractapi {
 /// 2. at a given record, walk up the class hierarchy starting from the record's
 /// dynamic type until APIRecord is reached.
 /// 3. given a (record, class) combination where 'class' is some base class of
-/// the dynamic type of 'node', call a user-overridable function to actually
+/// the dynamic type of 'record', call a user-overridable function to actually
 /// visit the record.
 ///
 /// These tasks are done by three groups of methods, respectively:
@@ -62,8 +62,8 @@ namespace extractapi {
 /// visitObjCInstancePropertyRecord()).
 ///
 /// This scheme guarantees that all visit*() calls for the same record
-/// are grouped together.  In other words, visit*() methods for different nodes
-/// are never interleaved.
+/// are grouped together.  In other words, visit*() methods for different
+/// records are never interleaved.
 ///
 /// Clients of this visitor should subclass the visitor (providing
 /// themselves as the template argument, using the curiously recurring
@@ -156,10 +156,13 @@ bool APISetVisitor<Derived>::traverseAPIRecord(const APIRecord *Record) {
     break;                                                                     \
   }
 #include "../APIRecords.inc"
-  case APIRecord::RK_Unknown:
+  case APIRecord::RK_Unknown: {
     TRY_TO(walkUpFromAPIRecord(static_cast<const APIRecord *>(Record)));
     break;
   }
+  default:
+    llvm_unreachable("API Record with uninstantiable kind");
+  }
   return true;
 }
 

clang/lib/ExtractAPI/API.cpp

@@ -61,7 +61,7 @@ void RecordContext::addToRecordChain(APIRecord *Record) const {
     return;
   }
 
-  Last->NextInContex = Record;
+  Last->NextInContext = Record;
   Last = Record;
 }
 

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

@@ -499,6 +499,8 @@ Object serializeSymbolKind(APIRecord::RecordKind RK, Language Lang) {
     Kind["identifier"] = AddLangPrefix("typealias");
     Kind["displayName"] = "Type Alias";
     break;
+  default:
+    llvm_unreachable("API Record with uninstantiable kind");
   }
 
   return Kind;
@@ -763,7 +765,7 @@ void SymbolGraphSerializer::serializeAPIRecord(const APIRecord *Record) {
   Obj["accessLevel"] = Record->Access.getAccess();
 
   ExtendedModule &Module = getModuleForCurrentSymbol();
-  // If the hierarchy has at leas one parent and child.
+  // If the hierarchy has at least one parent and child.
   if (Hierarchy.size() >= 2)
     serializeRelationship(MemberOf, Hierarchy.back(),
                           Hierarchy[Hierarchy.size() - 2], Module);
@@ -998,6 +1000,8 @@ void SymbolGraphSerializer::serializeSingleRecord(const APIRecord *Record) {
   case APIRecord::RK_Unknown:
     visitAPIRecord(Record);
     break;
+  default:
+    llvm_unreachable("API Record with uninstantiable kind");
   }
 }
 

clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp

@@ -59,12 +59,12 @@ TypedefUnderlyingTypeResolver::getSymbolReferenceForType(QualType Type,
 
     clang::index::generateUSRForDecl(TypeDecl, TypeUSR);
     if (auto *OwningModule = TypeDecl->getImportedOwningModule())
-      OwningModuleName = API.copyString(OwningModule->Name);
+      OwningModuleName = OwningModule->Name;
   } else {
     clang::index::generateUSRForType(Type, Context, TypeUSR);
   }
 
-  return {API.copyString(TypeName), API.copyString(TypeUSR), OwningModuleName};
+  return API.createSymbolReference(TypeName, TypeUSR, OwningModuleName);
 }
 
 std::string TypedefUnderlyingTypeResolver::getUSRForType(QualType Type) const {

clang/test/ExtractAPI/constructor_destructor.cpp

@@ -137,7 +137,7 @@ class Foo {
         "precise": "c:@S@Foo@F@Foo#"
       },
       "kind": {
-        "displayName": "Instance Method",
+        "displayName": "Constructor",
         "identifier": "c++.method"
       },
       "location": {
@@ -193,7 +193,7 @@ class Foo {
         "precise": "c:@S@Foo@F@~Foo#"
       },
       "kind": {
-        "displayName": "Instance Method",
+        "displayName": "Destructor",
         "identifier": "c++.method"
       },
       "location": {

clang/test/ExtractAPI/metadata_and_module.c

@@ -1,8 +1,8 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -extract-api --pretty-sgf --product-name=module -triple arm64-apple-macosx -x c-header %s -o %t/module.symbols.json -verify
 
-// RUN: Filecheck %s --input-file %t/module.symbols.json --check-prefix METADATA
-// RUN: Filecheck %s --input-file %t/module.symbols.json --check-prefix MOD
+// RUN: FileCheck %s --input-file %t/module.symbols.json --check-prefix METADATA
+// RUN: FileCheck %s --input-file %t/module.symbols.json --check-prefix MOD
 
 // expected-no-diagnostics
 

clang/test/ExtractAPI/objc_block.m

@@ -3,7 +3,7 @@
 // RUN:   -fblocks -triple arm64-apple-macosx -x objective-c-header %s -o %t/output.symbols.json -verify
 
 @interface Foo
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix NOPARAM
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix NOPARAM
 -(void)methodBlockNoParam:(void (^)())block;
 // NOPARAM-LABEL: "!testLabel": "c:objc(cs)Foo(im)methodBlockNoParam:"
 // NOPARAM:      "declarationFragments": [
@@ -88,7 +88,7 @@ -(void)methodBlockNoParam:(void (^)())block;
 // NOPARAM-NEXT:   ]
 // NOPARAM-NEXT: }
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix PARAM
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix PARAM
 -(void)methodBlockWithParam:(int (^)(int foo))block;
 // PARAM-LABEL: "!testLabel": "c:objc(cs)Foo(im)methodBlockWithParam:"
 // PARAM:      "declarationFragments": [
@@ -207,7 +207,7 @@ -(void)methodBlockWithParam:(int (^)(int foo))block;
 // PARAM-NEXT:   ]
 // PARAM-NEXT: }
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix MULTIPARAM
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix MULTIPARAM
 -(void)methodBlockWithMultipleParam:(int (^)(int foo, unsigned baz))block;
 // MULTIPARAM-LABEL: "!testLabel": "c:objc(cs)Foo(im)methodBlockWithMultipleParam:"
 // MULTIPARAM:      "declarationFragments": [
@@ -360,7 +360,7 @@ -(void)methodBlockWithMultipleParam:(int (^)(int foo, unsigned baz))block;
 // MULTIPARAM-NEXT:   ]
 // MULTIPARAM-NEXT: },
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix VARIADIC
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix VARIADIC
 -(void)methodBlockVariadic:(int (^)(int foo, ...))block;
 // VARIADIC-LABEL: "!testLabel": "c:objc(cs)Foo(im)methodBlockVariadic:"
 // VARIADIC:      "declarationFragments": [
@@ -480,7 +480,7 @@ -(void)methodBlockVariadic:(int (^)(int foo, ...))block;
 // VARIADIC-NEXT: },
 @end
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix FUNC
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix FUNC
 void func(int (^arg)(int foo));
 // FUNC-LABEL: "!testLabel": "c:@F@func"
 // FUNC:      "declarationFragments": [
@@ -587,7 +587,7 @@ -(void)methodBlockVariadic:(int (^)(int foo, ...))block;
 // FUNC-NEXT:   ]
 // FUNC-NEXT: },
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix GLOBAL
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix GLOBAL
 int (^global)(int foo);
 // GLOBAL-LABEL: "!testLabel": "c:@global"
 // GLOBAL:      "declarationFragments": [

clang/test/ExtractAPI/objc_category.m

@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -extract-api --pretty-sgf --emit-sgf-symbol-labels-for-testing \
-// RUN:   -triple arm64-apple-macosx -x objective-c-header %s -o - -verify | Filecheck %s
+// RUN:   -triple arm64-apple-macosx -x objective-c-header %s -o - -verify | FileCheck %s
 
 @protocol Protocol
 @end

clang/test/ExtractAPI/objc_external_category.m

@@ -28,7 +28,7 @@ @interface ExtInterface
     header "ExternalModule.h"
 }
 
-// RUN: Filecheck %s --input-file  %t/symbols/Module.symbols.json --check-prefix MOD
+// RUN: FileCheck %s --input-file  %t/symbols/Module.symbols.json --check-prefix MOD
 // MOD-NOT: "!testRelLabel": "memberOf $ c:objc(cs)ExtInterface(py)Property $ c:objc(cs)ExtInterface"
 // MOD-NOT: "!testRelLabel": "memberOf $ c:objc(cs)ExtInterface(im)InstanceMethod $ c:objc(cs)ExtInterface"
 // MOD-NOT: "!testRelLabel": "memberOf $ c:objc(cs)ExtInterface(cm)ClassMethod $ c:objc(cs)ExtInterface"
@@ -38,7 +38,7 @@ @interface ExtInterface
 // MOD-NOT: "!testLabel": "c:objc(cs)ExtInterface"
 // MOD-DAG: "!testLabel": "c:objc(cs)ModInterface"
 
-// RUN: Filecheck %s --input-file %t/symbols/[email protected] --check-prefix EXT
+// RUN: FileCheck %s --input-file %t/symbols/[email protected] --check-prefix EXT
 // EXT-DAG: "!testRelLabel": "memberOf $ c:objc(cs)ExtInterface(py)Property $ c:objc(cs)ExtInterface"
 // EXT-DAG: "!testRelLabel": "memberOf $ c:objc(cs)ExtInterface(im)InstanceMethod $ c:objc(cs)ExtInterface"
 // EXT-DAG: "!testRelLabel": "memberOf $ c:objc(cs)ExtInterface(cm)ClassMethod $ c:objc(cs)ExtInterface"

clang/test/ExtractAPI/objc_id_protocol.m

@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -extract-api --pretty-sgf --emit-sgf-symbol-labels-for-testing \
-// RUN:   -x objective-c-header -triple arm64-apple-macosx %s -o - -verify | Filecheck %s
+// RUN:   -x objective-c-header -triple arm64-apple-macosx %s -o - -verify | FileCheck %s
 
 @protocol MyProtocol
 @end

clang/test/ExtractAPI/objc_interface.m

@@ -5,7 +5,7 @@
 @protocol Protocol
 @end
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix SUPER
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix SUPER
 @interface Super <Protocol>
 // SUPER: "!testRelLabel": "conformsTo $ c:objc(cs)Super $ c:objc(pl)Protocol"
 // SUPER-LABEL: "!testLabel": "c:objc(cs)Super"
@@ -33,7 +33,7 @@ @interface Super <Protocol>
 // SUPER-NEXT:   "Super"
 // SUPER-NEXT: ]
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix PROP
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix PROP
 @property(readonly, getter=getProperty) unsigned Property;
 // PROP: "!testRelLabel": "memberOf $ c:objc(cs)Super(py)Property $ c:objc(cs)Super"
 // PROP: "!testLabel": "c:objc(cs)Super(py)Property"
@@ -99,7 +99,7 @@ @interface Super <Protocol>
 // PROP-NEXT:   "Property"
 // PROP-NEXT: ]
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix GET
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix GET
 + (id)getWithProperty:(unsigned) Property;
 // GET: "!testRelLabel": "memberOf $ c:objc(cs)Super(cm)getWithProperty: $ c:objc(cs)Super"
 // GET-LABEL: "!testLabel": "c:objc(cs)Super(cm)getWithProperty:"
@@ -185,7 +185,7 @@ + (id)getWithProperty:(unsigned) Property;
 // GET-NEXT:   "getWithProperty:"
 // GET-NEXT: ]
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix SET
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix SET
 - (void)setProperty:(unsigned) Property andOtherThing: (unsigned) Thing;
 // SET: "!testRelLabel": "memberOf $ c:objc(cs)Super(im)setProperty:andOtherThing: $ c:objc(cs)Super"
 // SET-LABEL: "!testLabel": "c:objc(cs)Super(im)setProperty:andOtherThing:"
@@ -317,11 +317,11 @@ - (void)setProperty:(unsigned) Property andOtherThing: (unsigned) Thing;
 // SET:   "title": "setProperty:andOtherThing:"
 @end
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix DERIVED
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix DERIVED
 @interface Derived : Super {
 // DERIVED: "!testRelLabel": "inheritsFrom $ c:objc(cs)Derived $ c:objc(cs)Super"
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix IVAR
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix IVAR
   char Ivar;
 // IVAR: "!testRelLabel": "memberOf $ c:objc(cs)Derived@Ivar $ c:objc(cs)Derived"
 // IVAR-LABEL: "!testLabel": "c:objc(cs)Derived@Ivar"

clang/test/ExtractAPI/objc_property.m

@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -extract-api --pretty-sgf --emit-sgf-symbol-labels-for-testing \
-// RUN:   -triple arm64-apple-macosx -x objective-c-header %s -o - -verify | Filecheck %s
+// RUN:   -triple arm64-apple-macosx -x objective-c-header %s -o - -verify | FileCheck %s
 
 @protocol Protocol
 @property(class) int myProtocolTypeProp;

clang/test/ExtractAPI/typedef.c

@@ -43,7 +43,7 @@ typedef int MyInt;
 // MYINT-NEXT: ],
 // MYINT: "type": "c:I"
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json --check-prefix BARPTR
+// RUN: FileCheck %s --input-file %t/output.symbols.json --check-prefix BARPTR
 typedef struct Bar *BarPtr;
 // BARPTR-LABEL: "!testLabel": "c:typedef.c@T@BarPtr"
 // BARPTR: "accessLevel": "public",
@@ -84,7 +84,7 @@ typedef struct Bar *BarPtr;
 // BARPTR-NEXT: ],
 // BARPTR: "type": "c:*$@S@Bar"
 
-// RUN: Filecheck %s --input-file %t/output.symbols.json
+// RUN: FileCheck %s --input-file %t/output.symbols.json
 void foo(BarPtr value);
 
 void baz(BarPtr *value);