[clang-doc] Fix typedef/using output.

[brettw]

Summary

Provides an initializer for the TypedefInfo.IsUsing member. Previously this member was uninitialized and would produce random output.

Adds the Description (code comments) to the bitcode reader/writer. Previously the typedef/using descriptions were lost during the bitcode round-trip. Adds a test for this.

[paulkirth]

Can you add a test that checks the IsUsing == false case? Otherwise LGTM modulo one small nit.

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

395–396

nit: I know this is in the style of the rest of the file, but I believe that code was written before LLVM allowed C++17. I think we can just return the reference from emplace_back() directly, right?

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

184

This seems to be a test for the IsUsing == true case, are there tests where it is false? I realize that's the default now, but I'm not sure it's covered.

[paulkirth]

LGTM.

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

395–396

oh, I didn't intend for you to update the whole file, just what was in your change, but thank you for the code clean up!

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

 }
 
 template <typename T> llvm::Expected<CommentInfo *> getCommentInfo(T I) {
   return llvm::createStringError(llvm::inconvertibleErrorCode(),
                                  "invalid type cannot contain CommentInfo");
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(FunctionInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(NamespaceInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(RecordInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(MemberTypeInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(EnumInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
+}
+
+template <> llvm::Expected<CommentInfo *> getCommentInfo(TypedefInfo *I) {
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected<CommentInfo *> getCommentInfo(CommentInfo *I) {
   I->Children.emplace_back(std::make_unique<CommentInfo>());
   return I->Children.back().get();
 }
 
 template <>
 llvm::Expected<CommentInfo *> getCommentInfo(std::unique_ptr<CommentInfo> &I) {
   return getCommentInfo(I.get());
 }
 
 // When readSubBlock encounters a TypeInfo sub-block, it calls addTypeInfo on

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

 }
 
 void ClangDocBitcodeWriter::emitBlock(const TypedefInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_TYPEDEF_BLOCK_ID);
   emitRecord(T.USR, TYPEDEF_USR);
   emitRecord(T.Name, TYPEDEF_NAME);
   for (const auto &N : T.Namespace)
     emitBlock(N, FieldId::F_namespace);
+  for (const auto &CI : T.Description)
+    emitBlock(CI);
   if (T.DefLoc)
     emitRecord(*T.DefLoc, TYPEDEF_DEFLOCATION);
   emitRecord(T.IsUsing, TYPEDEF_IS_USING);
   emitBlock(T.Underlying);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const FieldTypeInfo &T) {
   StreamSubBlockGuard Block(Stream, BI_FIELD_TYPE_BLOCK_ID);

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

struct TypedefInfo : public SymbolInfo {
   void merge(TypedefInfo &&I);
 
   TypeInfo Underlying;
 
   // Inidicates if this is a new C++ "using"-style typedef:
   //   using MyVector = std::vector<int>
   // False means it's a C-style typedef:
   //   typedef std::vector<int> MyVector;
-  bool IsUsing;
+  bool IsUsing = false;
 };
 
 struct BaseRecordInfo : public RecordInfo {
   BaseRecordInfo() : RecordInfo() {}
   BaseRecordInfo(SymbolID USR, StringRef Name, StringRef Path, bool IsVirtual,
                  AccessSpecifier Access, bool IsParent)
       : RecordInfo(USR, Name, Path), IsVirtual(IsVirtual), Access(Access),
         IsParent(IsParent) {}

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

 
 TEST(BitcodeTest, emitTypedefInfoBitcode) {
   TypedefInfo I;
   I.Name = "MyInt";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Underlying = TypeInfo("unsigned");
   I.IsUsing = true;
 
+  CommentInfo Top;
+  Top.Kind = "FullComment";
+
+  Top.Children.emplace_back(std::make_unique<CommentInfo>());
+  CommentInfo *BlankLine = Top.Children.back().get();
+  BlankLine->Kind = "ParagraphComment";
+  BlankLine->Children.emplace_back(std::make_unique<CommentInfo>());
+  BlankLine->Children.back()->Kind = "TextComment";
+
+  I.Description.emplace_back(std::move(Top));
+
   std::string WriteResult = writeInfo(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector<std::unique_ptr<Info>> ReadResults = readInfo(WriteResult, 1);
 
   CheckTypedefInfo(&I, InfoAsTypedef(ReadResults[0].get()));
+
+  // Check one with no IsUsing set, no description, and no definition location.
+  TypedefInfo I2;
+  I2.Name = "SomethingElse";
+  I2.IsUsing = false;
+  I2.Underlying = TypeInfo("int");
+
+  WriteResult = writeInfo(&I2);
+  EXPECT_TRUE(WriteResult.size() > 0);
+  ReadResults = readInfo(WriteResult, 1);
+  CheckTypedefInfo(&I2, InfoAsTypedef(ReadResults[0].get()));
 }
 
 TEST(SerializeTest, emitInfoWithCommentBitcode) {
   FunctionInfo F;
   F.Name = "F";
   F.ReturnType = TypeInfo("void");
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back(TypeInfo("int"), "I");