[clang-doc] Always emit the TagType for RecordInfo

[brettw]

Summary

Always emit the TagType for RecordInfo in YAML output. Previously this omitted the type for "struct", considering it the default. But records in C++ don't really have a default type so always emitting this is more clear.

Emit IsTypeDef in YAML. Previously this existed only in the Representation but was never written. Additionally, adds IsTypeDef to the record merge operation which was clearing it (all RecordInfo structures are merged with am empty RecordInfo during the reduce phase).

[paulkirth]

would you mind splitting this patch into separate patches? I'd like to keep the fix for the assertion in SmallString separate. the rest seems to make sense together. otherwise LGTM

[brettw]

Startup assert fix broken out into https://reviews.llvm.org/D131793

[paulkirth]

Can you update the summary to reflect what's in this change now + add a tag for clang-doc?

[clang-doc] Always emit the TagType for RecordInfo

I can land this and D131793 once the summaries are in order.

[brettw]

In D131739#3719634, @paulkirth wrote:

Can you update the summary to reflect what's in this change now + add a tag for clang-doc?

[clang-doc] Always emit the TagType for RecordInfo

I can land this and D131793 once the summaries are in order.

Done

clang-tools-extra/clang-doc/Representation.h

struct RecordInfo : public SymbolInfo {
   RecordInfo(SymbolID USR) : SymbolInfo(InfoType::IT_record, USR) {}
   RecordInfo(SymbolID USR, StringRef Name)
       : SymbolInfo(InfoType::IT_record, USR, Name) {}
   RecordInfo(SymbolID USR, StringRef Name, StringRef Path)
       : SymbolInfo(InfoType::IT_record, USR, Name, Path) {}
 
   void merge(RecordInfo &&I);
 
-  TagTypeKind TagType = TagTypeKind::TTK_Struct; // Type of this record
-                                                 // (struct, class, union,
-                                                 // interface).
-  bool IsTypeDef = false; // Indicates if record was declared using typedef
+  // Type of this record (struct, class, union, interface).
+  TagTypeKind TagType = TagTypeKind::TTK_Struct;
+
+  // Indicates if the record was declared using a typedef. Things like anonymous
+  // structs in a typedef:
+  //   typedef struct { ... } foo_t;
+  // are converted into records with the typedef as the Name + this flag set.
+  bool IsTypeDef = false;
+
   llvm::SmallVector<MemberTypeInfo, 4>
       Members;                             // List of info about record members.
   llvm::SmallVector<Reference, 4> Parents; // List of base/parent records
                                            // (does not include virtual
                                            // parents).
   llvm::SmallVector<Reference, 4>
       VirtualParents; // List of virtual base/parent records.
 

clang-tools-extra/clang-doc/Representation.cpp

void NamespaceInfo::merge(NamespaceInfo &&Other) {
   reduceChildren(ChildEnums, std::move(Other.ChildEnums));
   mergeBase(std::move(Other));
 }
 
 void RecordInfo::merge(RecordInfo &&Other) {
   assert(mergeable(Other));
   if (!TagType)
     TagType = Other.TagType;
+  IsTypeDef = IsTypeDef || Other.IsTypeDef;
   if (Members.empty())
     Members = std::move(Other.Members);
   if (Bases.empty())
     Bases = std::move(Other.Bases);
   if (Parents.empty())
     Parents = std::move(Other.Parents);
   if (VirtualParents.empty())
     VirtualParents = std::move(Other.VirtualParents);

clang-tools-extra/clang-doc/YAMLGenerator.cpp

 static void SymbolInfoMapping(IO &IO, SymbolInfo &I) {
   InfoMapping(IO, I);
   IO.mapOptional("DefLocation", I.DefLoc, Optional<Location>());
   IO.mapOptional("Location", I.Loc, llvm::SmallVector<Location, 2>());
 }
 
 static void RecordInfoMapping(IO &IO, RecordInfo &I) {
   SymbolInfoMapping(IO, I);
-  IO.mapOptional("TagType", I.TagType, clang::TagTypeKind::TTK_Struct);
+  IO.mapOptional("TagType", I.TagType);
+  IO.mapOptional("IsTypeDef", I.IsTypeDef, false);
   IO.mapOptional("Members", I.Members);
   IO.mapOptional("Bases", I.Bases);
   IO.mapOptional("Parents", I.Parents, llvm::SmallVector<Reference, 4>());
   IO.mapOptional("VirtualParents", I.VirtualParents,
                  llvm::SmallVector<Reference, 4>());
   IO.mapOptional("ChildRecords", I.ChildRecords, std::vector<Reference>());
   IO.mapOptional("ChildFunctions", I.ChildFunctions);
   IO.mapOptional("ChildEnums", I.ChildEnums);

clang-tools-extra/unittests/clang-doc/MergeTest.cpp

TEST(MergeTest, mergeNamespaceInfos) {
   assert(Actual);
   CheckNamespaceInfo(InfoAsNamespace(Expected.get()),
                      InfoAsNamespace(Actual.get().get()));
 }
 
 TEST(MergeTest, mergeRecordInfos) {
   RecordInfo One;
   One.Name = "r";
+  One.IsTypeDef = true;
   One.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   One.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
 
   One.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
   One.TagType = TagTypeKind::TTK_Class;
   One.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
   One.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
TEST(MergeTest, mergeRecordInfos) {
   Two.ChildEnums.back().Name = "TwoEnum";
 
   std::vector<std::unique_ptr<Info>> Infos;
   Infos.emplace_back(std::make_unique<RecordInfo>(std::move(One)));
   Infos.emplace_back(std::make_unique<RecordInfo>(std::move(Two)));
 
   auto Expected = std::make_unique<RecordInfo>();
   Expected->Name = "r";
+  Expected->IsTypeDef = true;
   Expected->Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   Expected->DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   Expected->Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   Expected->Members.emplace_back("int", "X", AccessSpecifier::AS_private);
   Expected->TagType = TagTypeKind::TTK_Class;
   Expected->Parents.emplace_back(EmptySID, "F", InfoType::IT_record);

clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

 )raw";
   EXPECT_EQ(Expected, Actual.str());
 }
 
 TEST(YAMLGeneratorTest, emitRecordYAML) {
   RecordInfo I;
   I.Name = "r";
   I.Path = "path/to/A";
+  I.IsTypeDef = true;
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("int", "path/to/int", "X",
                          AccessSpecifier::AS_private);
 
- Type:            Namespace
     Name:            'A'
 DefLocation:
   LineNumber:      10
   Filename:        'test.cpp'
 Location:
   - LineNumber:      12
     Filename:        'test.cpp'
 TagType:         Class
+IsTypeDef:       true
 Members:
   - Type:
       Name:            'int'
       Path:            'path/to/int'
     Name:            'X'
     Access:          Private
     Description:
       - Kind:            'FullComment'
         Children:
           - Kind:            'ParagraphComment'
             Children:
               - Kind:            'TextComment'
                 Text:            'Value of the thing.'
                 Name:            'ParagraphComment'
 Bases:
   - USR:             '0000000000000000000000000000000000000000'
     Name:            'F'
     Path:            'path/to/F'
+    TagType:         Struct
     Members:
       - Type:
           Name:            'int'
           Path:            'path/to/int'
         Name:            'N'
         Access:          Private
     ChildFunctions:
       - USR:             '0000000000000000000000000000000000000000'