[PATCH] D133732: [clang-doc] Support default args for functions.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Adds support for default arguments in the internal representation and reads 
these values from the source. Implements writing these values to YAML but does 
not implement this for the HTML or markdown outputs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133732

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -209,6 +209,8 @@
   I.ReturnType =
   TypeInfo(EmptySID, "void", InfoType::IT_default, "path/to/void");
   I.Params.emplace_back("int", "path/to/int", "P");
+  I.Params.emplace_back("double", "path/to/double", "D");
+  I.Params.back().DefaultValue = "2.0 * M_PI";
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
@@ -240,6 +242,11 @@
   Name:'int'
   Path:'path/to/int'
 Name:'P'
+  - Type:
+  Name:'double'
+  Path:'path/to/double'
+Name:'D'
+DefaultValue:'2.0 * M_PI'
 ReturnType:
   Type:
 Name:'void'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -511,7 +511,7 @@
   std::vector Args;
   Args.push_back("-fmodules-ts");
   ExtractInfosFromCodeWithArgs(R"raw(export module M;
-int moduleFunction(int x);
+int moduleFunction(int x, double d = 3.2 - 1.0);
 static int staticModuleFunction(int x);
 export double exportedModuleFunction(double y);)raw",
2, /*Public=*/true, Infos, Args);
@@ -523,6 +523,8 @@
   F.ReturnType = TypeInfo(EmptySID, "int", InfoType::IT_default);
   F.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back("int", "x");
+  F.Params.emplace_back("double", "d");
+  F.Params.back().DefaultValue = "3.2 - 1.0";
   F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(&ExpectedBWithFunction, BWithFunction);
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -109,6 +109,7 @@
 static void FieldTypeInfoMapping(IO &IO, FieldTypeInfo &I) {
   TypeInfoMapping(IO, I);
   IO.mapOptional("Name", I.Name, SmallString<16>());
+  IO.mapOptional("DefaultValue", I.DefaultValue, SmallString<16>());
 }
 
 static void InfoMapping(IO &IO, Info &I) {
@@ -183,6 +184,7 @@
   static void mapping(IO &IO, FieldTypeInfo &I) {
 TypeInfoMapping(IO, I);
 IO.mapOptional("Name", I.Name, SmallString<16>());
+IO.mapOptional("DefaultValue", I.DefaultValue, SmallString<16>());
   }
 };
 
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -10,6 +10,7 @@
 #include "BitcodeWriter.h"
 #include "clang/AST/Comment.h"
 #include "clang/Index/USRGeneration.h"
+#include "clang/Lex/Lexer.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/SHA1.h"
@@ -296,7 +297,7 @@
   }
 }
 
-auto& member = I.Members.emplace_back(
+auto &member = I.Members.emplace_back(
 F->getTypeSourceInfo()->getType().getAsString(), F->getNameAsString(),
 getFinalAccessSpecifier(Access, F->getAccessUnsafe()));
 populateMemberTypeInfo(member, F);
@@ -310,21 +311,31 @@
 
 static void parseParameters(FunctionInfo &I, const FunctionDecl *D) {
   for (const ParmVarDecl *P : D->parameters()) {
+FieldTypeInfo *FieldInfo = nullptr;
 if (const auto *T = getDeclForType(P->getOriginalType())) {
   if (const auto *N = dyn_cast(T)) {
-I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
-  InfoType::IT_enum, getInfoRelativePath(N),
-  P->getNameAsString());
-continue;
+FieldInfo = &I.Params.emplace_back(
+getU

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: phosek.
brettw added a project: clang-tools-extra.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Struct/class data members did not have the comments associated with them. This 
adds that information to the MemberTypeInfo class and emits it in the YAML. 
This does not update the frontends yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131298

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp


Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -192,6 +192,7 @@
 // the AS that shouldn't be part of the output. Even though AS_public is 
the
 // default in the struct, it should be displayed in the YAML output.
 IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
+IO.mapOptional("Description", I.Description);
   }
 };
 
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -29,6 +29,8 @@
 populateParentNamespaces(llvm::SmallVector &Namespaces,
  const T *D, bool &IsAnonymousNamespace);
 
+static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D);
+
 // A function to extract the appropriate relative path for a given info's
 // documentation. The path returned is a composite of the parent namespaces.
 //
@@ -293,9 +295,11 @@
 continue;
   }
 }
-I.Members.emplace_back(
+
+auto& member = I.Members.emplace_back(
 F->getTypeSourceInfo()->getType().getAsString(), F->getNameAsString(),
 getFinalAccessSpecifier(Access, F->getAccessUnsafe()));
+populateMemberTypeInfo(member, F);
   }
 }
 
@@ -431,6 +435,19 @@
   parseParameters(I, D);
 }
 
+static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) {
+  ASTContext& Context = D->getASTContext();
+  RawComment *Comment = Context.getRawCommentForDeclNoCache(D);
+  if (!Comment)
+return;
+
+  Comment->setAttached();
+  if (comments::FullComment* fc = Comment->parse(Context, nullptr, D)) {
+I.Description.emplace_back();
+parseFullComment(fc, I.Description.back());
+  }
+}
+
 static void
 parseBases(RecordInfo &I, const CXXRecordDecl *D, bool IsFileInRootDir,
bool PublicOnly, bool IsParent,
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -210,6 +210,8 @@
   // with value 0 to be used as the default.
   // (AS_public = 0, AS_protected = 1, AS_private = 2, AS_none = 3)
   AccessSpecifier Access = AccessSpecifier::AS_public;
+
+  std::vector Description; // Comment description of this field.
 };
 
 struct Location {
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -426,6 +426,8 @@
   emitBlock(T.Type, FieldId::F_type);
   emitRecord(T.Name, MEMBER_TYPE_NAME);
   emitRecord(T.Access, MEMBER_TYPE_ACCESS);
+  for (const auto &CI : T.Description)
+emitBlock(CI);
 }
 
 void ClangDocBitcodeWriter::emitBlock(const CommentInfo &I) {
Index: clang-tools-extra/clang-doc/BitcodeReader.cpp
===
--- clang-tools-extra/clang-doc/BitcodeReader.cpp
+++ clang-tools-extra/clang-doc/BitcodeReader.cpp
@@ -350,6 +350,11 @@
   return &I->Description.back();
 }
 
+template <> llvm::Expected getCommentInfo(MemberTypeInfo *I) {
+  I->Description.emplace_back();
+  return &I->Description.back();
+}
+
 template <> llvm::Expected getCommentInfo(EnumInfo *I) {
   I->Description.emplace_back();
   return &I->Description.back();


Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -192,6 +192,7 @@
 // the AS that shouldn't be part of the output. Even though AS_public is the
 // default in the struct, it should be displayed in the YAML output.
 IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
+IO.mapOptional("Description", I.Description);
   }
 };
 
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-d

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 450967.
brettw added a comment.
Herald added subscribers: usaxena95, arphaman.

Added some tests (with one open question).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -83,6 +83,19 @@
 
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
+
+  // Member documentation.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Members.back().Description.push_back(std::move(TopComment));
+
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
@@ -129,6 +142,14 @@
   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: ''
 Name:'F'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -80,6 +80,23 @@
   ASSERT_EQ(NumExpectedInfos, EmittedInfos.size());
 }
 
+// Constructs a comment definition as the parser would for one comment line.
+CommentInfo MakeOneLineCommentInfo(const std::string &Text) {
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+
+  return TopComment;
+}
+
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
@@ -124,6 +141,9 @@
   ExtractInfosFromCode(R"raw(class E {
 public:
   E() {}
+
+  // Some docs.
+  int value;
 protected:
   void ProtectedMethod();
 };
@@ -142,6 +162,8 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
 
   RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
@@ -172,6 +194,7 @@
 InfoType::IT_namespace);
   Method.Access = AccessSpecifier::AS_protected;
   Method.IsMethod = true;
+
   ExpectedRecordWithMethod.ChildFunctions.emplace_back(std::move(Method));
   CheckRecordInfo(&ExpectedRecordWithMethod, RecordWithMethod);
 
Index: clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
@@ -34,7 +34,12 @@
   return static_cast(I);
 }
 
-void CheckCommentInfo(CommentInfo &Expected, CommentInfo &Actual) {
+void CheckCommentInfo(const std::vector &Expected,
+  const std::vector &Actual);
+void CheckCommentInfo(const std::vector> &Expected,
+  const std::vector> &Actual);
+
+void CheckCommentInfo(const CommentInfo &Expected, const CommentInfo &Actual) {
   EXPECT_EQ(Expected.Kind, A

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 450976.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -83,6 +83,19 @@
 
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
+
+  // Member documentation.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Members.back().Description.push_back(std::move(TopComment));
+
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
@@ -129,6 +142,14 @@
   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: ''
 Name:'F'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -80,6 +80,23 @@
   ASSERT_EQ(NumExpectedInfos, EmittedInfos.size());
 }
 
+// Constructs a comment definition as the parser would for one comment line.
+CommentInfo MakeOneLineCommentInfo(const std::string &Text) {
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+
+  return TopComment;
+}
+
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
@@ -124,6 +141,9 @@
   ExtractInfosFromCode(R"raw(class E {
 public:
   E() {}
+
+  // Some docs.
+  int value;
 protected:
   void ProtectedMethod();
 };
@@ -142,6 +162,8 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
 
   RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
@@ -172,6 +194,7 @@
 InfoType::IT_namespace);
   Method.Access = AccessSpecifier::AS_protected;
   Method.IsMethod = true;
+
   ExpectedRecordWithMethod.ChildFunctions.emplace_back(std::move(Method));
   CheckRecordInfo(&ExpectedRecordWithMethod, RecordWithMethod);
 
Index: clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
@@ -34,7 +34,12 @@
   return static_cast(I);
 }
 
-void CheckCommentInfo(CommentInfo &Expected, CommentInfo &Actual) {
+void CheckCommentInfo(const std::vector &Expected,
+  const std::vector &Actual);
+void CheckCommentInfo(const std::vector> &Expected,
+  const std::vector> &Actual);
+
+void CheckCommentInfo(const CommentInfo &Expected, const CommentInfo &Actual) {
   EXPECT_EQ(Expected.Kind, Actual.Kind);
   EXPECT_EQ(Expected.Text, Actual.Text);
   EXPECT_EQ(Expected.Name, Actual.Name);
@@ -56,9 +61,21 @@

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.






Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:354
+template <> llvm::Expected getCommentInfo(MemberTypeInfo *I) {
+  I->Description.emplace_back();
+  return &I->Description.back();

paulkirth wrote:
> So, I see that this uses the same pattern as in other `getCommentInfo(...)` 
> API's, but I'm a bit confused by this.
> 
> Do we always grow the vector whenever `getCommentInfo(...)` is  called? I 
> would expect a `get...` function to be `const` and just return data. This 
> grows the `Description` vector on every call, which seems like an odd choice. 
> The pattern is also pervasive in BitcodeReader.cpp.
> 
> @phosek is this intentional? If `Description` exceeds capacity and reallocs 
> on `emplace_back`, then any reference it had returned would be invalidated, 
> right? Or am I missing something here re: the various `*TypeInfos` and how 
> clang-doc uses them?
You are correct that this always grows. These functions are only called once 
and that's when readSubBlock gets a block ID. The comment is filled into the 
returned array.

My question is why the comments on the info structs are arrays. I have only 
ever seen one top-level comment of type "FullComment" which can have many 
children.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:439
+static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) {
+  ASTContext& Context = D->getASTContext();
+  RawComment *Comment = Context.getRawCommentForDeclNoCache(D);

paulkirth wrote:
> I think we need to at least assert that `D != nullptr`. While there is only 
> one caller now, its likely there may be more in the future. Plus `nullptr` is 
> deref is never fun to debug.  
> 
> If getting a `nullptr` is expected to be a common input, then an early return 
> is also fine.
> 
> If you prefer, a few places in the codebase do both to maintain correct 
> behavior when asserts are disabled. Often they have a form along these lines:
> 
> ```
> if(!D){
>   assert(false && "Some error message");
>   return;
> }
> ```
> 
> Any of these would be fine, and I don't have a preference between them.
> 
I'm fine adding an assert to unblock the review.

But clang-doc is full of pointers and none of them are asserted on. Is this one 
special in some way?  To me, it seems weird to assert on this one random 
pointer. Is there some policy about this?

I follow the opposite theory that asserts on null pointers all over the place 
clutter the code and random null parameters are one of the easiest types of 
crashes to debug.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:440
+  ASTContext& Context = D->getASTContext();
+  RawComment *Comment = Context.getRawCommentForDeclNoCache(D);
+  if (!Comment)

paulkirth wrote:
> If the goal is to get the FullComment, then doesn't `getCommentForDecl(...) ` 
> handle that? This also seems like an area where we'd want to use caching if 
> we could.
I have no idea how this works, I copied this snipped from 
MapASTVisitor::getComment in Mapper.cpp



Comment at: clang-tools-extra/clang-doc/YAMLGenerator.cpp:195
 IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
+IO.mapOptional("Description", I.Description);
   }

paulkirth wrote:
> Can we get a comment describing this?
I don't think this needs a comment, a substantial part of this file is just 
adding these mappings between YAML keys and field names. The one above it has a 
comment only because of the non-obvious default value. If you feel strongly, 
please let me know what the comment should say.



Comment at: clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp:291
+/*
+TEST(BitcodeTest, emitMemberTypeInfo) {
+  MemberTypeInfo I;

Whoops, I'll delete this.



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:166
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" 
Some docs"));
   CheckRecordInfo(&ExpectedE, E);

I spent quite some time trying to get this to work and I'm stuck. If I copy the 
contents of the raw string above into a file and run clang-doc on it, I get a 
comment in the YAML output. But in this test I never get a comment and I don't 
know how this parsing environment is different than the "real" one.

I'm very confused by the result of `ExtractInfosFromCode` function in the first 
place. What are the 1st and 6th item which are never checked? Why does it emit 
two duplicate records for the `E` struct, one (Infos[2]) with a data member and 
no function and one (Infos[3]) with no data member and a function in it?

Any suggestions here would be appreciated.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 451001.
brettw marked an inline comment as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -83,6 +83,19 @@
 
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
+
+  // Member documentation.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Members.back().Description.push_back(std::move(TopComment));
+
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
@@ -129,6 +142,14 @@
   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: ''
 Name:'F'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -80,6 +80,23 @@
   ASSERT_EQ(NumExpectedInfos, EmittedInfos.size());
 }
 
+// Constructs a comment definition as the parser would for one comment line.
+CommentInfo MakeOneLineCommentInfo(const std::string &Text) {
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+
+  return TopComment;
+}
+
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
@@ -124,6 +141,9 @@
   ExtractInfosFromCode(R"raw(class E {
 public:
   E() {}
+
+  // Some docs.
+  int value;
 protected:
   void ProtectedMethod();
 };
@@ -142,6 +162,8 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
 
   RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
Index: clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
@@ -34,7 +34,12 @@
   return static_cast(I);
 }
 
-void CheckCommentInfo(CommentInfo &Expected, CommentInfo &Actual) {
+void CheckCommentInfo(const std::vector &Expected,
+  const std::vector &Actual);
+void CheckCommentInfo(const std::vector> &Expected,
+  const std::vector> &Actual);
+
+void CheckCommentInfo(const CommentInfo &Expected, const CommentInfo &Actual) {
   EXPECT_EQ(Expected.Kind, Actual.Kind);
   EXPECT_EQ(Expected.Text, Actual.Text);
   EXPECT_EQ(Expected.Name, Actual.Name);
@@ -56,9 +61,21 @@
   for (size_t Idx = 0; Idx < Actual.Args.size(); ++Idx)
 EXPECT_EQ(Expected.Args[Idx], Actual.Args[Idx]);
 
-  ASSERT_EQ(Expected.Children.size(), Actual.Children.size());
-  for (size_t Idx = 0; Idx < Actual.Children.size(); ++Idx)
-CheckCommentInfo

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw marked 2 inline comments as done.
brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:439
+static void populateMemberTypeInfo(MemberTypeInfo &I, const FieldDecl *D) {
+  ASTContext& Context = D->getASTContext();
+  RawComment *Comment = Context.getRawCommentForDeclNoCache(D);

paulkirth wrote:
> brettw wrote:
> > paulkirth wrote:
> > > I think we need to at least assert that `D != nullptr`. While there is 
> > > only one caller now, its likely there may be more in the future. Plus 
> > > `nullptr` is deref is never fun to debug.  
> > > 
> > > If getting a `nullptr` is expected to be a common input, then an early 
> > > return is also fine.
> > > 
> > > If you prefer, a few places in the codebase do both to maintain correct 
> > > behavior when asserts are disabled. Often they have a form along these 
> > > lines:
> > > 
> > > ```
> > > if(!D){
> > >   assert(false && "Some error message");
> > >   return;
> > > }
> > > ```
> > > 
> > > Any of these would be fine, and I don't have a preference between them.
> > > 
> > I'm fine adding an assert to unblock the review.
> > 
> > But clang-doc is full of pointers and none of them are asserted on. Is this 
> > one special in some way?  To me, it seems weird to assert on this one 
> > random pointer. Is there some policy about this?
> > 
> > I follow the opposite theory that asserts on null pointers all over the 
> > place clutter the code and random null parameters are one of the easiest 
> > types of crashes to debug.
> There's nothing special regarding this particular pointer, but such checks 
> are common in other parts of LLVM.
> 
> We use asserts liberally throughout the LLVM codebase 
> (https://llvm.org/docs/CodingStandards.html#assert-liberally) and asserting 
> that a pointer is valid is common. 
> 
> The other thing to keep in mind is that debug builds of LLVM are > 20GB, and 
> enabling asserts is a more desirable choice for most of our developers. That 
> probably isn't as bad for clang-doc, but I'd rather err on the side of 
> caution.
> 
> While I agree that right now it may be a bit strange to have a single assert, 
>  hopefully overtime the surrounding code will start to use such checks in a 
> way that is more consistent with the rest of the codebase.
> 
> But like I said, an early return is also fine IMO.
> 
Done


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 451299.
brettw marked an inline comment as done.
brettw added a comment.

I updated the comment test that's not currently working with a TODO.

There are some open questions in the code review about some bigger questions 
about this tool. Since this has been virtually abandoned for several years, I 
don't think anybody knows these answers and we're not going to figure out 
everything for this patch.

I think this patch makes things incrementally better and I'd like to land 
something soon so I can work on the next missing feature. I think over time we 
can build up knowledge about this tool and make it better on a wider scale. Can 
you please talk another look?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -83,6 +83,19 @@
 
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
+
+  // Member documentation.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Members.back().Description.push_back(std::move(TopComment));
+
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
@@ -129,6 +142,14 @@
   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: ''
 Name:'F'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -80,6 +80,23 @@
   ASSERT_EQ(NumExpectedInfos, EmittedInfos.size());
 }
 
+// Constructs a comment definition as the parser would for one comment line.
+CommentInfo MakeOneLineCommentInfo(const std::string &Text) {
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+
+  return TopComment;
+}
+
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
@@ -124,6 +141,9 @@
   ExtractInfosFromCode(R"raw(class E {
 public:
   E() {}
+
+  // Some docs.
+  int value;
 protected:
   void ProtectedMethod();
 };
@@ -142,6 +162,9 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  // TODO the data member should have the docstring on it:
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
 
   RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
Index: clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
@@ -34,7 +34,12 @@
   return static_cast(I);
 }
 
-void CheckCommentInfo(CommentInfo &Expected, CommentInfo &Actual) {
+void CheckCo

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 451632.
brettw added a comment.

New patch up.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -83,6 +83,19 @@
 
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
+
+  // Member documentation.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Members.back().Description.push_back(std::move(TopComment));
+
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
@@ -129,6 +142,14 @@
   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: ''
 Name:'F'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -80,6 +80,26 @@
   ASSERT_EQ(NumExpectedInfos, EmittedInfos.size());
 }
 
+// Constructs a comment definition as the parser would for one comment line.
+/* TODO uncomment this when the missing comment is fixed in emitRecordInfo and
+   the code that calls this is re-enabled.
+CommentInfo MakeOneLineCommentInfo(const std::string &Text) {
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = Text;
+
+  return TopComment;
+}
+*/
+
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
@@ -124,6 +144,9 @@
   ExtractInfosFromCode(R"raw(class E {
 public:
   E() {}
+
+  // Some docs.
+  int value;
 protected:
   void ProtectedMethod();
 };
@@ -142,6 +165,9 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  // TODO the data member should have the docstring on it:
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
 
   RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
Index: clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
@@ -34,7 +34,12 @@
   return static_cast(I);
 }
 
-void CheckCommentInfo(CommentInfo &Expected, CommentInfo &Actual) {
+void CheckCommentInfo(const std::vector &Expected,
+  const std::vector &Actual);
+void CheckCommentInfo(const std::vector> &Expected,
+  const std::vector> &Actual);
+
+void CheckCommentInfo(const CommentInfo &Expected, const CommentInfo &Actual) {
   EXPECT_EQ(Expected.Kind, Actual.Kind);
   EXPECT_EQ(Expected.Text, Actual.Text);
   EXPECT_EQ(Expected.Name, Actual.Name);
@@ -56,9 +61,21 @@
   for (size_t Idx = 0; Idx < Actual.Args.size(); ++Idx)
 EXPECT_EQ(Expected.Args[Idx

[PATCH] D136638: [clang-doc] Fix typedef/using output.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136638

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp


Index: clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
@@ -183,6 +183,17 @@
   I.Underlying = TypeInfo("unsigned");
   I.IsUsing = true;
 
+  CommentInfo Top;
+  Top.Kind = "FullComment";
+
+  Top.Children.emplace_back(std::make_unique());
+  CommentInfo *BlankLine = Top.Children.back().get();
+  BlankLine->Kind = "ParagraphComment";
+  BlankLine->Children.emplace_back(std::make_unique());
+  BlankLine->Children.back()->Kind = "TextComment";
+
+  I.Description.emplace_back(std::move(Top));
+
   std::string WriteResult = writeInfo(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector> ReadResults = readInfo(WriteResult, 1);
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -365,7 +365,7 @@
   //   using MyVector = std::vector
   // False means it's a C-style typedef:
   //   typedef std::vector MyVector;
-  bool IsUsing;
+  bool IsUsing = false;
 };
 
 struct BaseRecordInfo : public RecordInfo {
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -432,6 +432,8 @@
   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);
Index: clang-tools-extra/clang-doc/BitcodeReader.cpp
===
--- clang-tools-extra/clang-doc/BitcodeReader.cpp
+++ clang-tools-extra/clang-doc/BitcodeReader.cpp
@@ -392,6 +392,11 @@
   return &I->Description.back();
 }
 
+template <> llvm::Expected getCommentInfo(TypedefInfo *I) {
+  I->Description.emplace_back();
+  return &I->Description.back();
+}
+
 template <> llvm::Expected getCommentInfo(CommentInfo *I) {
   I->Children.emplace_back(std::make_unique());
   return I->Children.back().get();


Index: clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
@@ -183,6 +183,17 @@
   I.Underlying = TypeInfo("unsigned");
   I.IsUsing = true;
 
+  CommentInfo Top;
+  Top.Kind = "FullComment";
+
+  Top.Children.emplace_back(std::make_unique());
+  CommentInfo *BlankLine = Top.Children.back().get();
+  BlankLine->Kind = "ParagraphComment";
+  BlankLine->Children.emplace_back(std::make_unique());
+  BlankLine->Children.back()->Kind = "TextComment";
+
+  I.Description.emplace_back(std::move(Top));
+
   std::string WriteResult = writeInfo(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector> ReadResults = readInfo(WriteResult, 1);
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -365,7 +365,7 @@
   //   using MyVector = std::vector
   // False means it's a C-style typedef:
   //   typedef std::vector MyVector;
-  bool IsUsing;
+  bool IsUsing = false;
 };
 
 struct BaseRecordInfo : public RecordInfo {
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -432,6 +432,8 @@
   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);
Index: clang

[PATCH] D136638: [clang-doc] Fix typedef/using output.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 470598.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136638/new/

https://reviews.llvm.org/D136638

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
@@ -183,11 +183,33 @@
   I.Underlying = TypeInfo("unsigned");
   I.IsUsing = true;
 
+  CommentInfo Top;
+  Top.Kind = "FullComment";
+
+  Top.Children.emplace_back(std::make_unique());
+  CommentInfo *BlankLine = Top.Children.back().get();
+  BlankLine->Kind = "ParagraphComment";
+  BlankLine->Children.emplace_back(std::make_unique());
+  BlankLine->Children.back()->Kind = "TextComment";
+
+  I.Description.emplace_back(std::move(Top));
+
   std::string WriteResult = writeInfo(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector> 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) {
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -365,7 +365,7 @@
   //   using MyVector = std::vector
   // False means it's a C-style typedef:
   //   typedef std::vector MyVector;
-  bool IsUsing;
+  bool IsUsing = false;
 };
 
 struct BaseRecordInfo : public RecordInfo {
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -432,6 +432,8 @@
   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);
Index: clang-tools-extra/clang-doc/BitcodeReader.cpp
===
--- clang-tools-extra/clang-doc/BitcodeReader.cpp
+++ clang-tools-extra/clang-doc/BitcodeReader.cpp
@@ -368,28 +368,27 @@
 }
 
 template <> llvm::Expected getCommentInfo(FunctionInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(NamespaceInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(RecordInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(MemberTypeInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(EnumInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
+}
+
+template <> llvm::Expected getCommentInfo(TypedefInfo *I) {
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(CommentInfo *I) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136638: [clang-doc] Fix typedef/using output.

[Brett Wilson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7231c9966e52: [clang-doc] Fix typedef/using output. 
(authored by brettw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136638/new/

https://reviews.llvm.org/D136638

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
@@ -183,11 +183,33 @@
   I.Underlying = TypeInfo("unsigned");
   I.IsUsing = true;
 
+  CommentInfo Top;
+  Top.Kind = "FullComment";
+
+  Top.Children.emplace_back(std::make_unique());
+  CommentInfo *BlankLine = Top.Children.back().get();
+  BlankLine->Kind = "ParagraphComment";
+  BlankLine->Children.emplace_back(std::make_unique());
+  BlankLine->Children.back()->Kind = "TextComment";
+
+  I.Description.emplace_back(std::move(Top));
+
   std::string WriteResult = writeInfo(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector> 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) {
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -365,7 +365,7 @@
   //   using MyVector = std::vector
   // False means it's a C-style typedef:
   //   typedef std::vector MyVector;
-  bool IsUsing;
+  bool IsUsing = false;
 };
 
 struct BaseRecordInfo : public RecordInfo {
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -432,6 +432,8 @@
   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);
Index: clang-tools-extra/clang-doc/BitcodeReader.cpp
===
--- clang-tools-extra/clang-doc/BitcodeReader.cpp
+++ clang-tools-extra/clang-doc/BitcodeReader.cpp
@@ -368,28 +368,27 @@
 }
 
 template <> llvm::Expected getCommentInfo(FunctionInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(NamespaceInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(RecordInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(MemberTypeInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(EnumInfo *I) {
-  I->Description.emplace_back();
-  return &I->Description.back();
+  return &I->Description.emplace_back();
+}
+
+template <> llvm::Expected getCommentInfo(TypedefInfo *I) {
+  return &I->Description.emplace_back();
 }
 
 template <> llvm::Expected getCommentInfo(CommentInfo *I) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136894: Add clang-doc readme

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Adds a readme to clang-doc with an overview of the structure of the code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136894

Files:
  clang-tools-extra/clang-doc/README.txt


Index: clang-tools-extra/clang-doc/README.txt
===
--- /dev/null
+++ clang-tools-extra/clang-doc/README.txt
@@ -0,0 +1,60 @@
+==
+How clang-doc works internally
+==
+
+Clang-doc uses the "tooling" library which can run the compiler. It can take 
the files directly or
+it can extract the file list and the commands corresponding to each from a 
compile_commands.json
+file.
+
+The tooling library spins up threads and parses the compilation units in 
parallel. Clang-doc
+registers a callback to run on the AST of each unit.
+
+When the AST is known, the MapASTVisitor in Mapper.cpp is run on the AST. It 
has callbacks for the
+main AST nodes that clang-doc cares about. This is a very simple object that 
mostly calls into
+Serialize.cpp to generate the "representation" of the code. These are the 
various *Info structures
+like FunctionInfo and NamespaceInfo defined in Representation.h that 
correspond to each element of
+the code that might be documented.
+
+The representation from each execution thread is serialized to bitcode using 
BitcodeWriter.cpp. This
+is a custom bitcode defined in BitcodeWriter.h and is NOT regular LLVM bitcode 
IR. Watch out: the
+"Serialize.cpp" file is not related to this step though the name may imply it.
+
+These bitcode representations are then passed back to the main thread and 
deserialized back to a
+new copy of the representation in BitcodeReader.cpp. This round-trip through 
bitcode is used only to
+get the data out of the AST visitor (which is expected to return a byte 
stream) and is never saved
+or used for any other purpose. This round-trip adds significant complexity and 
we should consider
+passing the Representation object hierarchy back to the main thread 
out-of-band without
+serialization and deleting the bitcode representation.
+
+After deserialization, the various representation objects from each thread are 
merged/reduced into a
+single structure. This is necessary both to collect everything and to merge 
the declarations of
+things (of which there may be many) with the actual implementation. Many 
fields on a record can't be
+merged in the abstract (for example, a boolean field on two structures with 
two different values has
+no clear merged result). But this question can be resolved by thinking about 
the items as
+definitions and declarations and picking the value which would be present on 
the definition (usually
+the more specific or non-default value).
+
+After merging the final representation is passed to the output generator which 
writes the final
+files.
+
+==
+To add a new field
+==
+
+To add a new bit of information to the documentation that you extract from the 
AST:
+
+ 1. Add it to the appropriate *Info structure in Representation.h.
+
+ 2. Extract the information from the AST by adding to the code in 
Serialize.cpp and possibly
+Mapper.cpp. Save it to the Info structure used above.
+
+ 3. Write bitcode serialization code in BitcodeWriter.cpp for the new field.
+
+ 4. Write bitcode deserialization code in BitcodeReader.cpp.
+
+ 5. Add merging code in the merge() function for your Info structure (see 
above for advice).
+
+ 6. Update the backend(s) to use the new data.
+
+Note! Steps 3, 4, and especially 5 are easy to forget. Without all of these, 
the code may look like
+it works but you will only get default values out at the end.


Index: clang-tools-extra/clang-doc/README.txt
===
--- /dev/null
+++ clang-tools-extra/clang-doc/README.txt
@@ -0,0 +1,60 @@
+==
+How clang-doc works internally
+==
+
+Clang-doc uses the "tooling" library which can run the compiler. It can take the files directly or
+it can extract the file list and the commands corresponding to each from a compile_commands.json
+file.
+
+The tooling library spins up threads and parses the compilation units in parallel. Clang-doc
+registers a callback to run on the AST of each unit.
+
+When the AST is known, the MapASTVisitor in Mapper.cpp is run on the AST. It has callbacks for the
+main AST nodes that clang-doc cares about. This is a very simple object that mostly calls into
+Serialize.cpp to generate the "representation" of the code. These are the various *Info structures
+like FunctionInfo and NamespaceInfo defined in Representation.h that correspond to each element of
+the code that might be document

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:166
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" 
Some docs"));
   CheckRecordInfo(&ExpectedE, E);

paulkirth wrote:
> brettw wrote:
> > I spent quite some time trying to get this to work and I'm stuck. If I copy 
> > the contents of the raw string above into a file and run clang-doc on it, I 
> > get a comment in the YAML output. But in this test I never get a comment 
> > and I don't know how this parsing environment is different than the "real" 
> > one.
> > 
> > I'm very confused by the result of `ExtractInfosFromCode` function in the 
> > first place. What are the 1st and 6th item which are never checked? Why 
> > does it emit two duplicate records for the `E` struct, one (Infos[2]) with 
> > a data member and no function and one (Infos[3]) with no data member and a 
> > function in it?
> > 
> > Any suggestions here would be appreciated.
> So I also wanted to understand why. `MakeOneLinecCmmentIInfo()` is not used 
> elsewhere in clang-doc, and I don't think it was ever tested.
> 
> Looking at the body it doesn't use it's input parameter at all. 
> 
> There may also be an issue w/ how it constructs the `CommentInfo`.
> 
> Leaving this as a TODO is fine. I don't think this is something we're going 
> to solve in this patch, but if you can also add a TODO to `MakeOneLine 
> CommentInfo`, it would be appreciated.
The problem isn't with MakeOneLineCommentInfo (I fixed the parameter usage) but 
rather that the records generated in the test has no comments at all. The 
current test code (with the line commented out) expects that the comment is 
completely missing, which is the only way the test will pass now.

Since nobody else uses it, I commented it out and referenced the calling code.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131298: [clang-doc] Read docstrings for record members

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 451882.
brettw marked 2 inline comments as done.
brettw added a comment.

New patch up with requested changes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131298/new/

https://reviews.llvm.org/D131298

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -83,6 +83,19 @@
 
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
+
+  // Member documentation.
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = "Value of the thing.";
+  I.Members.back().Description.push_back(std::move(TopComment));
+
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
@@ -129,6 +142,14 @@
   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: ''
 Name:'F'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -80,6 +80,26 @@
   ASSERT_EQ(NumExpectedInfos, EmittedInfos.size());
 }
 
+// Constructs a comment definition as the parser would for one comment line.
+/* TODO uncomment this when the missing comment is fixed in emitRecordInfo and
+   the code that calls this is re-enabled.
+CommentInfo MakeOneLineCommentInfo(const std::string &Text) {
+  CommentInfo TopComment;
+  TopComment.Kind = "FullComment";
+  TopComment.Children.emplace_back(std::make_unique());
+
+  CommentInfo *Brief = TopComment.Children.back().get();
+  Brief->Kind = "ParagraphComment";
+
+  Brief->Children.emplace_back(std::make_unique());
+  Brief->Children.back()->Kind = "TextComment";
+  Brief->Children.back()->Name = "ParagraphComment";
+  Brief->Children.back()->Text = Text;
+
+  return TopComment;
+}
+*/
+
 // Test serialization of namespace declarations.
 TEST(SerializeTest, emitNamespaceInfo) {
   EmittedInfoList Infos;
@@ -124,6 +144,9 @@
   ExtractInfosFromCode(R"raw(class E {
 public:
   E() {}
+
+  // Some docs.
+  int value;
 protected:
   void ProtectedMethod();
 };
@@ -142,6 +165,9 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  // TODO the data member should have the docstring on it:
+  //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
 
   RecordInfo *RecordWithEConstructor = InfoAsRecord(Infos[2].get());
Index: clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
+++ clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
@@ -34,7 +34,12 @@
   return static_cast(I);
 }
 
-void CheckCommentInfo(CommentInfo &Expected, CommentInfo &Actual) {
+void CheckCommentInfo(const std::vector &Expected,
+  const std::vector &Actual);
+void CheckCommentInfo(const std::vector> &Expected,
+  const std::vector> &Actual);
+
+void CheckCommentInfo(const CommentInfo &Expected, const CommentInfo &Actual) {
   EXPECT_EQ(Expected.Kind, Actual.Kind);
   EXPECT_EQ(Expected.Text, Actual.Text);
   EXPECT_EQ(Expected.Name, Actual.Name);
@@ -56,9 +61,21 @@
   for (size_t Idx = 0; I

[PATCH] D131739: Fix some clang-doc issues.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

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.

  

Fix crash on startup when generating HTML code. SmallString (surprisingly) 
can't handle self-assignment which was happening because parent_path returns a 
string piece into its argument. This avoids the self-assignment by using 
another intermediate string.

  

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).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131739

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -76,6 +76,7 @@
   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"});
@@ -136,6 +137,7 @@
   - LineNumber:  12
 Filename:'test.cpp'
 TagType: Class
+IsTypeDef:   true
 Members:
   - Type:
   Name:'int'
@@ -154,6 +156,7 @@
   - USR: ''
 Name:'F'
 Path:'path/to/F'
+TagType: Struct
 Members:
   - Type:
   Name:'int'
Index: clang-tools-extra/unittests/clang-doc/MergeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MergeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MergeTest.cpp
@@ -78,6 +78,7 @@
 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"});
@@ -119,6 +120,7 @@
 
   auto Expected = std::make_unique();
   Expected->Name = "r";
+  Expected->IsTypeDef = true;
   Expected->Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   Expected->DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -127,7 +127,8 @@
 
 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());
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -348,10 +348,15 @@
 
   void merge(RecordInfo &&I);
 
-  TagTypeKind TagType = TagTypeKind::TTK_Struct; // Type of this r

[PATCH] D131793: Fix assert on startup in clang-doc

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

When using `clang-doc --format=html` it will crash on startup because of an 
assertion doing a self-assignment of a `SmallString`. This patch removes the 
self-assignment by creating an intermediate copy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131793

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131739: Fix some clang-doc issues.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 452227.
brettw edited the summary of this revision.
brettw added a comment.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131739/new/

https://reviews.llvm.org/D131739

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp


Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -76,6 +76,7 @@
   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"});
@@ -136,6 +137,7 @@
   - LineNumber:  12
 Filename:'test.cpp'
 TagType: Class
+IsTypeDef:   true
 Members:
   - Type:
   Name:'int'
@@ -154,6 +156,7 @@
   - USR: ''
 Name:'F'
 Path:'path/to/F'
+TagType: Struct
 Members:
   - Type:
   Name:'int'
Index: clang-tools-extra/unittests/clang-doc/MergeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MergeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MergeTest.cpp
@@ -78,6 +78,7 @@
 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"});
@@ -119,6 +120,7 @@
 
   auto Expected = std::make_unique();
   Expected->Name = "r";
+  Expected->IsTypeDef = true;
   Expected->Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   Expected->DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -127,7 +127,8 @@
 
 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());
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -348,10 +348,15 @@
 
   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
   Members; // List of info about record 
members.
   llvm::SmallVector Parents; // List of base/parent records
Index: clang-tools-extra/clang-doc/Representation.cpp
===
--- clang-tools-extra/clang-doc/Representation.cpp
+++ clang-tools-extra/clang-doc/Representation.cpp
@@ -222,6 +222,7 @@
   assert(mergeable(Other));
   if (!TagType)
 TagType = Other.TagType;
+  IsTypeDef = IsTypeDef || Other.IsTypeDef;
   if (Members.empty())
 Members = std::move(Other.Members);
   if (Bases.empty())


Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -76,6 +76,7 @@
   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"});
@@ -136,6 +137,7 @@
   - LineNumber:  12
 Filename:'test.cpp'
 TagType:

[PATCH] D131793: [clang-doc] Fix assert on startup

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.

In D131793#3719619 , @paulkirth wrote:

> This is LGTM, but can you add a tag to the summary title/commit message? ie. 
> `[clang-doc] Fix assert on startup in clang-doc`

Done


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131793/new/

https://reviews.llvm.org/D131793

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131739: [clang-doc] Always emit the TagType for RecordInfo

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131739/new/

https://reviews.llvm.org/D131739

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133732: [clang-doc] Support default args for functions.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 460538.
brettw marked 4 inline comments as done.
brettw added a comment.

There is no usage change for this. It just adds another field to the YAML.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133732/new/

https://reviews.llvm.org/D133732

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -209,6 +209,8 @@
   I.ReturnType =
   TypeInfo(EmptySID, "void", InfoType::IT_default, "path/to/void");
   I.Params.emplace_back("int", "path/to/int", "P");
+  I.Params.emplace_back("double", "path/to/double", "D");
+  I.Params.back().DefaultValue = "2.0 * M_PI";
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
@@ -240,6 +242,11 @@
   Name:'int'
   Path:'path/to/int'
 Name:'P'
+  - Type:
+  Name:'double'
+  Path:'path/to/double'
+Name:'D'
+DefaultValue:'2.0 * M_PI'
 ReturnType:
   Type:
 Name:'void'
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -511,7 +511,7 @@
   std::vector Args;
   Args.push_back("-fmodules-ts");
   ExtractInfosFromCodeWithArgs(R"raw(export module M;
-int moduleFunction(int x);
+int moduleFunction(int x, double d = 3.2 - 1.0);
 static int staticModuleFunction(int x);
 export double exportedModuleFunction(double y);)raw",
2, /*Public=*/true, Infos, Args);
@@ -523,6 +523,8 @@
   F.ReturnType = TypeInfo(EmptySID, "int", InfoType::IT_default);
   F.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back("int", "x");
+  F.Params.emplace_back("double", "d");
+  F.Params.back().DefaultValue = "3.2 - 1.0";
   F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(&ExpectedBWithFunction, BWithFunction);
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -109,6 +109,7 @@
 static void FieldTypeInfoMapping(IO &IO, FieldTypeInfo &I) {
   TypeInfoMapping(IO, I);
   IO.mapOptional("Name", I.Name, SmallString<16>());
+  IO.mapOptional("DefaultValue", I.DefaultValue, SmallString<16>());
 }
 
 static void InfoMapping(IO &IO, Info &I) {
@@ -183,6 +184,7 @@
   static void mapping(IO &IO, FieldTypeInfo &I) {
 TypeInfoMapping(IO, I);
 IO.mapOptional("Name", I.Name, SmallString<16>());
+IO.mapOptional("DefaultValue", I.DefaultValue, SmallString<16>());
   }
 };
 
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -10,6 +10,7 @@
 #include "BitcodeWriter.h"
 #include "clang/AST/Comment.h"
 #include "clang/Index/USRGeneration.h"
+#include "clang/Lex/Lexer.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/SHA1.h"
@@ -310,21 +311,31 @@
 
 static void parseParameters(FunctionInfo &I, const FunctionDecl *D) {
   for (const ParmVarDecl *P : D->parameters()) {
+FieldTypeInfo *FieldInfo = nullptr;
 if (const auto *T = getDeclForType(P->getOriginalType())) {
   if (const auto *N = dyn_cast(T)) {
-I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
-  InfoType::IT_enum, getInfoRelativePath(N),
-  P->getNameAsString());
-continue;
+FieldInfo = &I.Params.emplace_back(
+getUSRForDecl(N), N->getNameAsString(), InfoType::IT_enum,
+getInfoRelativePath(N), P->getNameAsString());
   } else if (const auto *N = dyn_cast(T)) {
-I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
-  InfoType::IT_record, getInfoRelativePath(N),
-  P->getNameAsString());
-continue;
+FieldInfo = &I.Params.emplace_back(
+getUSRForDecl(N), N->getNameAsString(), InfoType::IT_record,
+getInfoRelat

[PATCH] D134055: [clang-doc] Export more enum information

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Add support for explicitly typed enums:

  enum Foo : unsigned { ... };

to the internal representation and to the YAML output.

  

Add support for getting the value of an enum constant, as well as accessing the 
original expression that produced it. This changes the YAML output of enums 
from an array of strings for the enum members to an array of dictionaries. 
These dictionaries now report the name, value, and original expression.

  

The markdown and HTML outputs are unchanged, they still output the name from 
the new enhanced internal schema.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134055

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -256,7 +256,11 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
-TEST(YAMLGeneratorTest, emitEnumYAML) {
+// Tests the equivalent of:
+// namespace A {
+// enum e { X };
+// }
+TEST(YAMLGeneratorTest, emitSimpleEnumYAML) {
   EnumInfo I;
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
@@ -265,7 +269,7 @@
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
-  I.Scoped = true;
+  I.Scoped = false;
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -286,9 +290,42 @@
 Location:
   - LineNumber:  12
 Filename:'test.cpp'
+Members:
+  - Name:'X'
+Value:   '0'
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+// Tests the equivalent of:
+// enum class e : short { X = FOO_BAR + 2 };
+TEST(YAMLGeneratorTest, enumTypedScopedEnumYAML) {
+  EnumInfo I;
+  I.Name = "e";
+
+  I.Members.emplace_back("X", llvm::APSInt::get(-9876), "FOO_BAR + 2");
+  I.Scoped = true;
+  I.BaseType = TypeInfo("short");
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'e'
 Scoped:  true
+BaseType:
+  Type:
+Name:'short'
 Members:
-  - 'X'
+  - Name:'X'
+Value:   '-9876'
+Expr:'FOO_BAR + 2'
 ...
 )raw";
   EXPECT_EQ(Expected, Actual.str());
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -266,8 +266,8 @@
   EnumInfo E;
   E.Name = "E";
   E.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  E.Members.emplace_back("X");
-  E.Members.emplace_back("Y");
+  E.Members.emplace_back("X", llvm::APSInt::get(0));
+  E.Members.emplace_back("Y", llvm::APSInt::get(1));
   ExpectedNamespaceWithEnum.ChildEnums.emplace_back(std::move(E));
   CheckNamespaceInfo(&ExpectedNamespaceWithEnum, NamespaceWithEnum);
 
@@ -277,8 +277,8 @@
   G.Name = "G";
   G.Scoped = true;
   G.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  G.Members.emplace_back("A");
-  G.Members.emplace_back("B");
+  G.Members.emplace_back("A", llvm::APSInt::get(0));
+  G.Members.emplace_back("B", llvm::APSInt::get(1));
   ExpectedNamespaceWithScopedEnum.ChildEnums.emplace_back(std::move(G));
   CheckNamespaceInfo(&ExpectedNamespaceWithScopedEnum, NamespaceWithScopedEnum);
 }
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -14,6 +14,7 @@
 
 using namespace clang::doc;
 
+// These define YAML traits for decoding the listed values within a vector.
 LLVM_YAML_IS_SEQUENCE_VECTOR(FieldTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(MemberTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(Reference)
@@ -21,6 +22,7 @@
 LLVM_YAML_IS_SEQUENCE_VECTOR(CommentInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(Enum

[PATCH] D134055: [clang-doc] Export enum type and value information.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 460882.
brettw marked 2 inline comments as done.
brettw retitled this revision from "[clang-doc] Export more enum information" 
to "[clang-doc] Export enum type and value information.".
brettw edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134055/new/

https://reviews.llvm.org/D134055

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -256,7 +256,11 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
-TEST(YAMLGeneratorTest, emitEnumYAML) {
+// Tests the equivalent of:
+// namespace A {
+// enum e { X };
+// }
+TEST(YAMLGeneratorTest, emitSimpleEnumYAML) {
   EnumInfo I;
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
@@ -265,7 +269,7 @@
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
-  I.Scoped = true;
+  I.Scoped = false;
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -286,9 +290,42 @@
 Location:
   - LineNumber:  12
 Filename:'test.cpp'
+Members:
+  - Name:'X'
+Value:   '0'
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+// Tests the equivalent of:
+// enum class e : short { X = FOO_BAR + 2 };
+TEST(YAMLGeneratorTest, enumTypedScopedEnumYAML) {
+  EnumInfo I;
+  I.Name = "e";
+
+  I.Members.emplace_back("X", llvm::APSInt::get(-9876), "FOO_BAR + 2");
+  I.Scoped = true;
+  I.BaseType = TypeInfo("short");
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'e'
 Scoped:  true
+BaseType:
+  Type:
+Name:'short'
 Members:
-  - 'X'
+  - Name:'X'
+Value:   '-9876'
+Expr:'FOO_BAR + 2'
 ...
 )raw";
   EXPECT_EQ(Expected, Actual.str());
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -266,8 +266,8 @@
   EnumInfo E;
   E.Name = "E";
   E.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  E.Members.emplace_back("X");
-  E.Members.emplace_back("Y");
+  E.Members.emplace_back("X", llvm::APSInt::get(0));
+  E.Members.emplace_back("Y", llvm::APSInt::get(1));
   ExpectedNamespaceWithEnum.ChildEnums.emplace_back(std::move(E));
   CheckNamespaceInfo(&ExpectedNamespaceWithEnum, NamespaceWithEnum);
 
@@ -277,8 +277,8 @@
   G.Name = "G";
   G.Scoped = true;
   G.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  G.Members.emplace_back("A");
-  G.Members.emplace_back("B");
+  G.Members.emplace_back("A", llvm::APSInt::get(0));
+  G.Members.emplace_back("B", llvm::APSInt::get(1));
   ExpectedNamespaceWithScopedEnum.ChildEnums.emplace_back(std::move(G));
   CheckNamespaceInfo(&ExpectedNamespaceWithScopedEnum, NamespaceWithScopedEnum);
 }
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -14,6 +14,7 @@
 
 using namespace clang::doc;
 
+// These define YAML traits for decoding the listed values within a vector.
 LLVM_YAML_IS_SEQUENCE_VECTOR(FieldTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(MemberTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(Reference)
@@ -21,6 +22,7 @@
 LLVM_YAML_IS_SEQUENCE_VECTOR(CommentInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(EnumValueInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(BaseRecordInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::SmallString<16>)
@@ -226,10 +228,22 @@
   }
 };
 
+template <> struct MappingTraits {
+  static void mapping(IO &IO, EnumValueInfo &I) {
+IO.mapOptional("Name", I.Name);
+
+SmallString<16> ValueStr;
+I.Value.toString(ValueStr);
+IO.mapOptional("Value", ValueStr);
+IO.mapOptional("Expr", I.Va

[PATCH] D134055: [clang-doc] Export enum type and value information.

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:53
 
+llvm::Error decodeRecord(const Record &R, llvm::APSInt &Field, llvm::StringRef 
Blob) {
+  auto ByteWidth = R[0];

paulkirth wrote:
> do you need to do all of this? APSInt already supports to/from string 
> methods, as well as converting to/from integers. can you use that here and in 
> the writer to avoid some complexity?
I don't think converting to an integer is a good idea because people sometimes 
use min/max values for enum values and since this could be signed or unsigned, 
it gets kind of complicated.

Serializing a number as a string to bitcode also seemed wrong to me.

The simplest thing would be to store the value as a string in the EnumValueInfo 
and then always treat this as a string from then on. If you want it simplified, 
I think that's the thing to do. But I thought you would want the numeric value 
stored in clang-doc's "ast" because some backends may want to treat this as a 
number, check its signed/unsignedness, etc. I happy to change this if you want.



Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:221
 return decodeRecord(R, I->Loc, Blob);
-  case ENUM_MEMBER:
-return decodeRecord(R, I->Members, Blob);

paulkirth wrote:
> Don't you still need this? if it's now unused, we should probably remove it 
> from the RecordId enum too.
I think this is only used for properties and not the child members, so it isn't 
used. I removed the enum.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:185
 
+SmallString<128> getSourceCode(const Decl* D, const SourceRange& R) {
+  return Lexer::getSourceText(

paulkirth wrote:
> We normally try to avoid returning small string by value. see 
> https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallstring-h
> 
> Normally in the case that we need to return a string from a method, we just 
> use std::string. In other cases, it may be more appropriate to use an output 
> parameter instead of a return.
I was just copying some other code in this file, I changed this to std::string.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:316
+  for (const EnumConstantDecl *E : D->enumerators()) {
+SmallString<128> ValueExpr;
+if (const Expr* InitExpr = E->getInitExpr())

paulkirth wrote:
> why was 128 chosen? Aren't we storing it into a `SmallString<16>` in 
> `EnumValueInfo`? is there some external reason that we expect this to be the 
> right size? 
> 
> Do you have an idea for how long these are likely to be? if we expect them to 
> be large or completely unpredictable, it may make more sense to use a 
> `std::string` and avoid wasting stack space.
> 
>  
I changed this and the EnumValueInfo struct to std::string. I think the usage 
of SmallString in these records is over the top but I was trying to copy the 
existing style.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134055/new/

https://reviews.llvm.org/D134055

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134055: [clang-doc] Add support for explicitly typed enums

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 461259.
brettw marked an inline comment as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134055/new/

https://reviews.llvm.org/D134055

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -256,7 +256,11 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
-TEST(YAMLGeneratorTest, emitEnumYAML) {
+// Tests the equivalent of:
+// namespace A {
+// enum e { X };
+// }
+TEST(YAMLGeneratorTest, emitSimpleEnumYAML) {
   EnumInfo I;
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
@@ -265,7 +269,7 @@
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
-  I.Scoped = true;
+  I.Scoped = false;
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -286,9 +290,42 @@
 Location:
   - LineNumber:  12
 Filename:'test.cpp'
+Members:
+  - Name:'X'
+Value:   '0'
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+// Tests the equivalent of:
+// enum class e : short { X = FOO_BAR + 2 };
+TEST(YAMLGeneratorTest, enumTypedScopedEnumYAML) {
+  EnumInfo I;
+  I.Name = "e";
+
+  I.Members.emplace_back("X", "-9876", "FOO_BAR + 2");
+  I.Scoped = true;
+  I.BaseType = TypeInfo("short");
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'e'
 Scoped:  true
+BaseType:
+  Type:
+Name:'short'
 Members:
-  - 'X'
+  - Name:'X'
+Value:   '-9876'
+Expr:'FOO_BAR + 2'
 ...
 )raw";
   EXPECT_EQ(Expected, Actual.str());
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -266,8 +266,8 @@
   EnumInfo E;
   E.Name = "E";
   E.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  E.Members.emplace_back("X");
-  E.Members.emplace_back("Y");
+  E.Members.emplace_back("X", "0");
+  E.Members.emplace_back("Y", "1");
   ExpectedNamespaceWithEnum.ChildEnums.emplace_back(std::move(E));
   CheckNamespaceInfo(&ExpectedNamespaceWithEnum, NamespaceWithEnum);
 
@@ -277,8 +277,8 @@
   G.Name = "G";
   G.Scoped = true;
   G.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  G.Members.emplace_back("A");
-  G.Members.emplace_back("B");
+  G.Members.emplace_back("A", "0");
+  G.Members.emplace_back("B", "1");
   ExpectedNamespaceWithScopedEnum.ChildEnums.emplace_back(std::move(G));
   CheckNamespaceInfo(&ExpectedNamespaceWithScopedEnum, NamespaceWithScopedEnum);
 }
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -14,6 +14,7 @@
 
 using namespace clang::doc;
 
+// These define YAML traits for decoding the listed values within a vector.
 LLVM_YAML_IS_SEQUENCE_VECTOR(FieldTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(MemberTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(Reference)
@@ -21,6 +22,7 @@
 LLVM_YAML_IS_SEQUENCE_VECTOR(CommentInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(EnumValueInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(BaseRecordInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::SmallString<16>)
@@ -226,10 +228,19 @@
   }
 };
 
+template <> struct MappingTraits {
+  static void mapping(IO &IO, EnumValueInfo &I) {
+IO.mapOptional("Name", I.Name);
+IO.mapOptional("Value", I.Value);
+IO.mapOptional("Expr", I.ValueExpr, SmallString<16>());
+  }
+};
+
 template <> struct MappingTraits {
   static void mapping(IO &IO, EnumInfo &I) {
 SymbolInfoMapping(IO, I);
 IO.mapOptional("Scoped", I.Scoped, false);
+IO.mapOptional("BaseType", I.BaseType);
 IO.mapOptional("Members", I.Members);
   }
 };
Index: clang-tools-extra/clang-doc/Serialize.cpp
=

[PATCH] D134055: [clang-doc] Add support for explicitly typed enums

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:53
 
+llvm::Error decodeRecord(const Record &R, llvm::APSInt &Field, llvm::StringRef 
Blob) {
+  auto ByteWidth = R[0];

paulkirth wrote:
> brettw wrote:
> > paulkirth wrote:
> > > do you need to do all of this? APSInt already supports to/from string 
> > > methods, as well as converting to/from integers. can you use that here 
> > > and in the writer to avoid some complexity?
> > I don't think converting to an integer is a good idea because people 
> > sometimes use min/max values for enum values and since this could be signed 
> > or unsigned, it gets kind of complicated.
> > 
> > Serializing a number as a string to bitcode also seemed wrong to me.
> > 
> > The simplest thing would be to store the value as a string in the 
> > EnumValueInfo and then always treat this as a string from then on. If you 
> > want it simplified, I think that's the thing to do. But I thought you would 
> > want the numeric value stored in clang-doc's "ast" because some backends 
> > may want to treat this as a number, check its signed/unsignedness, etc. I 
> > happy to change this if you want.
> Those are fair points, and I think I misread/misunderstood a bit of what's 
> going on here.
> 
> As for encoding/decoding integers,  BitcodeWriter already has integer 
> support, so if you need to convert to/from those, it should already work, 
> right?
> 
> Regardless, you may want to look at the bitcode reader/writer in llvm to see 
> how they serialize/deserialize APInt, as their implementation seems a bit 
> more straightforward IMO.
> 
> https://github.com/llvm/llvm-project/blob/9050a59c6678789831b7286b8b68d18b966c4694/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L1636
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L2520
> https://github.com/llvm/llvm-project/blob/9050a59c6678789831b7286b8b68d18b966c4694/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L2843
> https://github.com/llvm/llvm-project/blob/9050a59c6678789831b7286b8b68d18b966c4694/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L2743
I just converted everything to a string which allowed most of this code to be 
deleted. I don't think any theoretical benefit of keeping the APSInt is worth 
the complexity.



Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:71
+
+  llvm::SmallVector AsWords;
+  AsWords.resize(WordWidth);

paulkirth wrote:
> You can avoid the resize w/ SmallVector(Size) constructor, right?
This is now deleted.



Comment at: clang-tools-extra/clang-doc/Representation.h:425
+  // constant. This will be empty for implicit enumeration values.
+  std::string ValueExpr;
+};

paulkirth wrote:
> Sorry to nitpick, but SmallString was the correct choice to use in this type. 
> We avoid its use as a return value, because it tends to be brittle, and stops 
> us from assigning into arbitrary sized SmallStrings.  In the struct, it's a 
> reasonable choice, especially if you expect most uses to be small.  for these 
> expressions, I expect most to either be the number itself, or some shift 
> operation, so SmallString<16> was probably more than sufficient.
In the common case there will be will be empty, so std::string actually seems 
more efficient. Also, I personally think the current quantity of SmallString in 
this code is over-optimization.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134055/new/

https://reviews.llvm.org/D134055

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134225: [clang-doc] Centralize TypeInfo creation.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Several different places in the code had similar computations for the 
parameters that were eventually passed to the TypeInfo constructor.

  

This centralizes that code in one function, and allows passing TypeInfo to the 
various other *Info structures that need it.

  

Remove some "auto" types and replace with the real type for getting 
declarations. This was making some duplicate checking difficult to see.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134225

Files:
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp

Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -224,12 +224,34 @@
   return hashUSR(USR);
 }
 
-static RecordDecl *getDeclForType(const QualType &T) {
+static TagDecl *getTagDeclForType(const QualType &T) {
+  if (const TagDecl *D = T->getAsTagDecl())
+return D->getDefinition();
+  return nullptr;
+}
+
+static RecordDecl *getRecordDeclForType(const QualType &T) {
   if (const RecordDecl *D = T->getAsRecordDecl())
 return D->getDefinition();
   return nullptr;
 }
 
+TypeInfo getTypeInfoForType(const QualType &T) {
+  if (const TagDecl *TD = getTagDeclForType(T)) {
+InfoType IT;
+if (dyn_cast(TD)) {
+  IT = InfoType::IT_enum;
+} else if (dyn_cast(TD)) {
+  IT = InfoType::IT_record;
+} else {
+  IT = InfoType::IT_default;
+}
+return TypeInfo(Reference(getUSRForDecl(TD), TD->getNameAsString(),
+  IT, getInfoRelativePath(TD)));
+  }
+  return TypeInfo(Reference(T.getAsString()));
+}
+
 static bool isPublic(const clang::AccessSpecifier AS,
  const clang::Linkage Link) {
   if (AS == clang::AccessSpecifier::AS_private)
@@ -286,28 +308,14 @@
   for (const FieldDecl *F : D->fields()) {
 if (!shouldSerializeInfo(PublicOnly, /*IsInAnonymousNamespace=*/false, F))
   continue;
-if (const auto *T = getDeclForType(F->getTypeSourceInfo()->getType())) {
-  // Use getAccessUnsafe so that we just get the default AS_none if it's not
-  // valid, as opposed to an assert.
-  if (const auto *N = dyn_cast(T)) {
-I.Members.emplace_back(
-getUSRForDecl(T), N->getNameAsString(), InfoType::IT_enum,
-getInfoRelativePath(N), F->getNameAsString(),
-getFinalAccessSpecifier(Access, N->getAccessUnsafe()));
-continue;
-  } else if (const auto *N = dyn_cast(T)) {
-I.Members.emplace_back(
-getUSRForDecl(T), N->getNameAsString(), InfoType::IT_record,
-getInfoRelativePath(N), F->getNameAsString(),
-getFinalAccessSpecifier(Access, N->getAccessUnsafe()));
-continue;
-  }
-}
 
-auto& member = I.Members.emplace_back(
-F->getTypeSourceInfo()->getType().getAsString(), F->getNameAsString(),
+// Use getAccessUnsafe so that we just get the default AS_none if it's not
+// valid, as opposed to an assert.
+MemberTypeInfo &NewMember = I.Members.emplace_back(
+getTypeInfoForType(F->getTypeSourceInfo()->getType()),
+F->getNameAsString(),
 getFinalAccessSpecifier(Access, F->getAccessUnsafe()));
-populateMemberTypeInfo(member, F);
+populateMemberTypeInfo(NewMember, F);
   }
 }
 
@@ -325,27 +333,12 @@
 
 static void parseParameters(FunctionInfo &I, const FunctionDecl *D) {
   for (const ParmVarDecl *P : D->parameters()) {
-FieldTypeInfo *FieldInfo = nullptr;
-if (const auto *T = getDeclForType(P->getOriginalType())) {
-  if (const auto *N = dyn_cast(T)) {
-FieldInfo = &I.Params.emplace_back(
-getUSRForDecl(N), N->getNameAsString(), InfoType::IT_enum,
-getInfoRelativePath(N), P->getNameAsString());
-  } else if (const auto *N = dyn_cast(T)) {
-FieldInfo = &I.Params.emplace_back(
-getUSRForDecl(N), N->getNameAsString(), InfoType::IT_record,
-getInfoRelativePath(N), P->getNameAsString());
-  }
-  // Otherwise fall through to the default case below.
-}
-
-if (!FieldInfo) {
-  FieldInfo = &I.Params.emplace_back(P->getOriginalType().getAsString(),
- P->getNameAsString());
-}
+FieldTypeInfo& FieldInfo = I.Params.emplace_back(
+getTypeInfoForType(P->getOriginalType()),
+P->getNameAsString());
 
 if (const Expr *DefaultArg = P->getDefaultArg()) {
-  FieldInfo->DefaultValue = getSourceCode(D, DefaultArg->getSourceRange());
+  FieldInfo.DefaultValue = getSourceCode(D, DefaultArg->getSourceRange());
 }
   }
 }
@@ -363,14 +356,14 @@
   const Templa

[PATCH] D134055: [clang-doc] Add support for explicitly typed enums

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 461366.
brettw added a comment.

Clang-formatted


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134055/new/

https://reviews.llvm.org/D134055

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -256,7 +256,11 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
-TEST(YAMLGeneratorTest, emitEnumYAML) {
+// Tests the equivalent of:
+// namespace A {
+// enum e { X };
+// }
+TEST(YAMLGeneratorTest, emitSimpleEnumYAML) {
   EnumInfo I;
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
@@ -265,7 +269,7 @@
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
-  I.Scoped = true;
+  I.Scoped = false;
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -286,9 +290,42 @@
 Location:
   - LineNumber:  12
 Filename:'test.cpp'
+Members:
+  - Name:'X'
+Value:   '0'
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
+// Tests the equivalent of:
+// enum class e : short { X = FOO_BAR + 2 };
+TEST(YAMLGeneratorTest, enumTypedScopedEnumYAML) {
+  EnumInfo I;
+  I.Name = "e";
+
+  I.Members.emplace_back("X", "-9876", "FOO_BAR + 2");
+  I.Scoped = true;
+  I.BaseType = TypeInfo("short");
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'e'
 Scoped:  true
+BaseType:
+  Type:
+Name:'short'
 Members:
-  - 'X'
+  - Name:'X'
+Value:   '-9876'
+Expr:'FOO_BAR + 2'
 ...
 )raw";
   EXPECT_EQ(Expected, Actual.str());
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -266,8 +266,8 @@
   EnumInfo E;
   E.Name = "E";
   E.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  E.Members.emplace_back("X");
-  E.Members.emplace_back("Y");
+  E.Members.emplace_back("X", "0");
+  E.Members.emplace_back("Y", "1");
   ExpectedNamespaceWithEnum.ChildEnums.emplace_back(std::move(E));
   CheckNamespaceInfo(&ExpectedNamespaceWithEnum, NamespaceWithEnum);
 
@@ -277,8 +277,8 @@
   G.Name = "G";
   G.Scoped = true;
   G.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  G.Members.emplace_back("A");
-  G.Members.emplace_back("B");
+  G.Members.emplace_back("A", "0");
+  G.Members.emplace_back("B", "1");
   ExpectedNamespaceWithScopedEnum.ChildEnums.emplace_back(std::move(G));
   CheckNamespaceInfo(&ExpectedNamespaceWithScopedEnum, NamespaceWithScopedEnum);
 }
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -14,6 +14,7 @@
 
 using namespace clang::doc;
 
+// These define YAML traits for decoding the listed values within a vector.
 LLVM_YAML_IS_SEQUENCE_VECTOR(FieldTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(MemberTypeInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(Reference)
@@ -21,6 +22,7 @@
 LLVM_YAML_IS_SEQUENCE_VECTOR(CommentInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(EnumValueInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(BaseRecordInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::SmallString<16>)
@@ -226,10 +228,19 @@
   }
 };
 
+template <> struct MappingTraits {
+  static void mapping(IO &IO, EnumValueInfo &I) {
+IO.mapOptional("Name", I.Name);
+IO.mapOptional("Value", I.Value);
+IO.mapOptional("Expr", I.ValueExpr, SmallString<16>());
+  }
+};
+
 template <> struct MappingTraits {
   static void mapping(IO &IO, EnumInfo &I) {
 SymbolInfoMapping(IO, I);
 IO.mapOptional("Scoped", I.Scoped, false);
+IO.mapOptional("BaseType", I.BaseType);
 IO.mapOptional("Members", I.Members);
   }
 };
Index: clang-tools-extra/clang-doc/Serialize.cpp

[PATCH] D134225: [clang-doc] Centralize TypeInfo creation.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 461397.
brettw marked an inline comment as done.
brettw added a comment.

Comment addressed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134225/new/

https://reviews.llvm.org/D134225

Files:
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp

Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -224,12 +224,35 @@
   return hashUSR(USR);
 }
 
-static RecordDecl *getDeclForType(const QualType &T) {
+static TagDecl *getTagDeclForType(const QualType &T) {
+  if (const TagDecl *D = T->getAsTagDecl())
+return D->getDefinition();
+  return nullptr;
+}
+
+static RecordDecl *getRecordDeclForType(const QualType &T) {
   if (const RecordDecl *D = T->getAsRecordDecl())
 return D->getDefinition();
   return nullptr;
 }
 
+TypeInfo getTypeInfoForType(const QualType &T) {
+  const TagDecl *TD = getTagDeclForType(T);
+  if (!TD)
+return TypeInfo(Reference(T.getAsString()));
+
+  InfoType IT;
+  if (dyn_cast(TD)) {
+IT = InfoType::IT_enum;
+  } else if (dyn_cast(TD)) {
+IT = InfoType::IT_record;
+  } else {
+IT = InfoType::IT_default;
+  }
+  return TypeInfo(Reference(getUSRForDecl(TD), TD->getNameAsString(), IT,
+getInfoRelativePath(TD)));
+}
+
 static bool isPublic(const clang::AccessSpecifier AS,
  const clang::Linkage Link) {
   if (AS == clang::AccessSpecifier::AS_private)
@@ -286,28 +309,14 @@
   for (const FieldDecl *F : D->fields()) {
 if (!shouldSerializeInfo(PublicOnly, /*IsInAnonymousNamespace=*/false, F))
   continue;
-if (const auto *T = getDeclForType(F->getTypeSourceInfo()->getType())) {
-  // Use getAccessUnsafe so that we just get the default AS_none if it's not
-  // valid, as opposed to an assert.
-  if (const auto *N = dyn_cast(T)) {
-I.Members.emplace_back(
-getUSRForDecl(T), N->getNameAsString(), InfoType::IT_enum,
-getInfoRelativePath(N), F->getNameAsString(),
-getFinalAccessSpecifier(Access, N->getAccessUnsafe()));
-continue;
-  } else if (const auto *N = dyn_cast(T)) {
-I.Members.emplace_back(
-getUSRForDecl(T), N->getNameAsString(), InfoType::IT_record,
-getInfoRelativePath(N), F->getNameAsString(),
-getFinalAccessSpecifier(Access, N->getAccessUnsafe()));
-continue;
-  }
-}
 
-auto& member = I.Members.emplace_back(
-F->getTypeSourceInfo()->getType().getAsString(), F->getNameAsString(),
+// Use getAccessUnsafe so that we just get the default AS_none if it's not
+// valid, as opposed to an assert.
+MemberTypeInfo &NewMember = I.Members.emplace_back(
+getTypeInfoForType(F->getTypeSourceInfo()->getType()),
+F->getNameAsString(),
 getFinalAccessSpecifier(Access, F->getAccessUnsafe()));
-populateMemberTypeInfo(member, F);
+populateMemberTypeInfo(NewMember, F);
   }
 }
 
@@ -325,27 +334,11 @@
 
 static void parseParameters(FunctionInfo &I, const FunctionDecl *D) {
   for (const ParmVarDecl *P : D->parameters()) {
-FieldTypeInfo *FieldInfo = nullptr;
-if (const auto *T = getDeclForType(P->getOriginalType())) {
-  if (const auto *N = dyn_cast(T)) {
-FieldInfo = &I.Params.emplace_back(
-getUSRForDecl(N), N->getNameAsString(), InfoType::IT_enum,
-getInfoRelativePath(N), P->getNameAsString());
-  } else if (const auto *N = dyn_cast(T)) {
-FieldInfo = &I.Params.emplace_back(
-getUSRForDecl(N), N->getNameAsString(), InfoType::IT_record,
-getInfoRelativePath(N), P->getNameAsString());
-  }
-  // Otherwise fall through to the default case below.
-}
-
-if (!FieldInfo) {
-  FieldInfo = &I.Params.emplace_back(P->getOriginalType().getAsString(),
- P->getNameAsString());
-}
+FieldTypeInfo &FieldInfo = I.Params.emplace_back(
+getTypeInfoForType(P->getOriginalType()), P->getNameAsString());
 
 if (const Expr *DefaultArg = P->getDefaultArg()) {
-  FieldInfo->DefaultValue = getSourceCode(D, DefaultArg->getSourceRange());
+  FieldInfo.DefaultValue = getSourceCode(D, DefaultArg->getSourceRange());
 }
   }
 }
@@ -363,14 +356,14 @@
   const TemplateDecl *D = Ty->getTemplateName().getAsTemplateDecl();
   I.Parents.emplace_back(getUSRForDecl(D), B.getType().getAsString(),
  InfoType::IT_record);
-} else if (const RecordDecl *P = getDeclForType(B.getType()))
+} else if (const RecordDecl *P = getRecordDeclForType(B.getType()))
   I.Parents.emplace_back(getUSRForDecl(P), P->getNameAsString(),
  InfoType::IT_record, getInfoRelativePath(P));
 else
   I.Parents.emplace_back(B.g

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
Herald added a project: All.
brettw requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The *Info object (for the copy of the AST") constructors had many duplicated 
variants. Many of the variants seemed to be in an attempt to avoid default 
arguments. But default arguments are not prohibited and using them allows most 
of the variants to be removed which improves readability.

Remove the IsInGlobalNamespace flag on a Reference. This is set when the path 
is empty, and only read once in the HTML generator with the identical 
condition. The constructor cleanup exposed a problem where this was set to 
false when the constructor with no path was used, but true when the path was 
set to empty.

There should be no observable change with the exception that 
IsInGlobalNamespace is no longer emitted in YAML.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134235

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/GeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -82,7 +82,7 @@
   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",
+  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
  AccessSpecifier::AS_private);
 
   // Member documentation.
@@ -102,7 +102,7 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().ChildFunctions.emplace_back();
   I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back("int", "path/to/int", "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
@@ -174,7 +174,6 @@
 Parents:
   - Type:Record
 Name:'F'
-IsInGlobalNamespace: true
 VirtualParents:
   - Type:Record
 Name:'G'
@@ -206,10 +205,10 @@
 
   I.Access = AccessSpecifier::AS_none;
 
-  I.ReturnType =
-  TypeInfo(EmptySID, "void", InfoType::IT_default, "path/to/void");
-  I.Params.emplace_back("int", "path/to/int", "P");
-  I.Params.emplace_back("double", "path/to/double", "D");
+  I.ReturnType = TypeInfo(
+  Reference(EmptySID, "void", InfoType::IT_default, "path/to/void"));
+  I.Params.emplace_back(TypeInfo("int", "path/to/int"), "P");
+  I.Params.emplace_back(TypeInfo("double", "path/to/double"), "D");
   I.Params.back().DefaultValue = "2.0 * M_PI";
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
@@ -335,9 +334,9 @@
   FunctionInfo I;
   I.Name = "f";
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
-  I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
-  I.Params.emplace_back("int", "I");
-  I.Params.emplace_back("int", "J");
+  I.ReturnType = TypeInfo("void");
+  I.Params.emplace_back(TypeInfo("int"), "I");
+  I.Params.emplace_back(TypeInfo("int"), "J");
   I.Access = AccessSpecifier::AS_none;
 
   CommentInfo Top;
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -119,7 +119,7 @@
   NamespaceInfo ExpectedBWithFunction(EmptySID);
   FunctionInfo F;
   F.Name = "f";
-  F.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  F.ReturnType = TypeInfo("void");
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   F.Namespace.emplace_back(EmptySID, "B", InfoType::IT_namespace);
   F.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
@@ -165,7 +165,8 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Loc

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added a subscriber: arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Read typedef and "using" type alias declarations and serialize into the 
internal structures. Emit this information in the YAML output. The HTML and MD 
generators are unchanged.

Separate out the logic to create the parent namespace or record object and 
insert the newly created child into it. This logic was previously duplicated 
for every "info" type and is now shared.

To help this, a struct containing the child vectors was separated out so 
children can be added generically and without having too many templates.

A small change was made to populateParentNamespaces() to allow using types that 
aren't themselves DeclContexts (typedefs are the first example of this).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134371

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -330,6 +330,30 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(YAMLGeneratorTest, enumTypedefYAML) {
+  TypedefInfo I;
+  I.Name = "MyUsing";
+  I.Underlying = TypeInfo("int");
+  I.IsUsing = true;
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'MyUsing'
+Underlying:
+  Name:'int'
+IsUsing: true
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 TEST(YAMLGeneratorTest, emitCommentYAML) {
   FunctionInfo I;
   I.Name = "f";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -59,6 +59,10 @@
   bool VisitRecordDecl(const RecordDecl *D) { return mapDecl(D); }
 
   bool VisitEnumDecl(const EnumDecl *D) { return mapDecl(D); }
+
+  bool VisitTypedefDecl(const TypedefDecl *D) { return mapDecl(D); }
+
+  bool VisitTypeAliasDecl(const TypeAliasDecl *D) { return mapDecl(D); }
 };
 
 void ExtractInfosFromCode(StringRef Code, size_t NumExpectedInfos, bool Public,
@@ -591,5 +595,32 @@
   CheckNamespaceInfo(&ExpectedParentB, ParentB);
 }
 
+TEST(SerializeTests, emitTypedefs) {
+  EmittedInfoList Infos;
+  ExtractInfosFromCode("typedef int MyInt; using MyDouble = double;", 2,
+   /*Public=*/false, Infos);
+
+  // First info will be the global namespace with the typedef in it.
+  NamespaceInfo *GlobalNS1 = InfoAsNamespace(Infos[0].get());
+  ASSERT_EQ(1u, GlobalNS1->ChildTypedefs.size());
+
+  const TypedefInfo &FirstTD = GlobalNS1->ChildTypedefs[0];
+  EXPECT_EQ("MyInt", FirstTD.Name);
+  EXPECT_FALSE(FirstTD.IsUsing);
+  EXPECT_EQ("int", FirstTD.Underlying.Type.Name);
+
+  // The second will be another global namespace with the using in it (the
+  // global namespace is duplicated because the items haven't been merged at the
+  // serialization phase of processing).
+  NamespaceInfo *GlobalNS2 = InfoAsNamespace(Infos[1].get());
+  ASSERT_EQ(1u, GlobalNS2->ChildTypedefs.size());
+
+  // Second is the "using" typedef.
+  const TypedefInfo &SecondTD = GlobalNS2->ChildTypedefs[0];
+  EXPECT_EQ("MyDouble", SecondTD.Name);
+  EXPECT_TRUE(SecondTD.IsUsing);
+  EXPECT_EQ("double", SecondTD.Underlying.Type.Name);
+}
+
 } // namespace doc
 } // end namespace clang
Index: clang-tools-extra/unittests/clang-doc/ClangDocTest.h
===
--- clang-tools-extra/unittests/clang-doc/ClangDocTest.h
+++ clang-tools-extra/unittests/clang-doc/ClangDocTest.h
@@ -27,6 +27,7 @@
 RecordInfo *InfoAsRecord(Info *I);
 FunctionInfo *InfoAsFunction(Info *I);
 EnumInfo *InfoAsEnum(Info *I);
+T

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via Phabricator via cfe-commits]

brettw marked an inline comment as done.
brettw added a comment.

First, for the mechanical part of the change, this code implement multiple 
constructors:

  Info() = default;
  Info(InfoType IT) : IT(IT) {}
  Info(InfoType IT, SymbolID USR) : USR(USR), IT(IT) {}
  Info(InfoType IT, SymbolID USR, StringRef Name)
      : USR(USR), IT(IT), Name(Name) {}
  Info(InfoType IT, SymbolID USR, StringRef Name, StringRef Path)

is not common practice. This is an extremely verbose and error-prone way of 
implementing default arguments. It seems obvious to me that the one constructor 
that implements the exact same behavior is much more clear. Most of this patch 
is doing this mechanical change for these objects.

There are a few cases in this patch where I removed some constructors that took 
some "other" subset of parameters.

  Reference() = default;
  Reference(llvm::StringRef Name) : Name(Name) {}
  Reference(llvm::StringRef Name, StringRef Path)
      : Name(Name), Path(Path), IsInGlobalNamespace(Path.empty()) {}
  Reference(SymbolID USR, StringRef Name, InfoType IT)
      : USR(USR), Name(Name), RefType(IT) {}
  Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
      : USR(USR), Name(Name), RefType(IT), Path(Path),
        IsInGlobalNamespace(Path.empty()) {}

Working on this code, I found the number of constructors with different 
parameters to make things more difficult to follow. It's harder to figure out 
what exactly a line of code is doing when there are so many variants you have 
to match against. It's also more difficult to figure out how to create one of 
these objects when you have to read so many variants to see what to call. This 
mental overhead may not be obvious when just looking at a diff, especially the 
test code where you may see an extra empty SID parameter inserted here and 
there. This new one tries to have a single constructor with default arguments 
from the right, with the exception of the "TypeInfo" that creates a single 
named type with no SymbolID because that's by far the most common variant in 
tests.

With this Reference constructor in particular, all of these variants hide a 
bug. If I didn't call that bug out in the change description, would you have 
seen it? Having a standard constructor makes this much easier to follow. 
Changing several callers to manually specify an empty SymbolID() seems like a 
small price to pay (I think this is the only case where any calls get "more 
complicated."

I tried to standardize on a single way to construct these objects. As an 
example, the TypeInfo is just a wrapper around a Reference. Reference has many 
constructors, not all of which are a subset of the others. Does this mean 
TypeInfo should get these same 5 constructor variants? The author said no, they 
picked 3 of the variants and implemented TypeInfo constructors, and used 
different names for some of the parameters which makes it even harder to 
follow. Continuing down the chain, FieldTypeInfo derives from TypeInfo, and 
MemberTypeInfo derives from FieldTypeInfo. Does this mean they should get the 
same 5 variants, plus initializers for their own values? I would say no. If 
these derived classes needed to specify the Reference information, they should 
take a Reference as the first parameter (they don't all need to do this today).

There's a problem with the current object model. The FieldTypeInfo and 
MemberTypeInfo derive from TypeInfo. But I think that this is wrong. The 
FieldTypeInfo isn't the type information for a field, it's really a "Field" and 
a MemberTypeInfo isn't the type info for a member, it's the "Member" itself. 
Conceptually, FieldTypeInfo should be a FieldInfo and have a struct `TypeInfo 
Type;` rather than using an "is a" relationship and deriving from a type. Given 
that, this object naming and constructor "should" be:

  MemberInfo(TypeInfo Type, StringRef Name, AccessSpecifier Access = Public)

This patch gets us closer to this, where we still have the weird "is a" 
derivation but the constructors match what I think they should be:

  MemberTypeInfo() = default;
  MemberTypeInfo(const TypeInfo &TI, StringRef Name, AccessSpecifier Access)

The parameters that define the implementation details of how the type is 
represented don't belong on this class: if the caller wants to create a type, 
they should create it using one of the appropriate TypeInfo constructors rather 
than each variant being duplicated here.




Comment at: clang-tools-extra/clang-doc/Representation.h:165
-  : Type(Type, Field, IT) {}
-  TypeInfo(SymbolID Type, StringRef Field, InfoType IT, StringRef Path)
-  : Type(Type, Field, IT, Path) {}

haowei wrote:
> The clean up here prevents setting a customized SymbolID for TypeInfo, which 
> becomes a problem in the HTMLGeneratorTest, in which TypeInfo is constructed 
> with an constant EmptySID. While the outcome is the same, it is not 
> semantically equivalent.  
It does not prevent sett

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via Phabricator via cfe-commits]

brettw marked an inline comment as done.
brettw added a comment.

If you have more questions, I can schedule a meeting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134235/new/

https://reviews.llvm.org/D134235

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 462281.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134235/new/

https://reviews.llvm.org/D134235

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/GeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -82,7 +82,7 @@
   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",
+  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
  AccessSpecifier::AS_private);
 
   // Member documentation.
@@ -102,7 +102,7 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().ChildFunctions.emplace_back();
   I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back("int", "path/to/int", "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
@@ -174,7 +174,6 @@
 Parents:
   - Type:Record
 Name:'F'
-IsInGlobalNamespace: true
 VirtualParents:
   - Type:Record
 Name:'G'
@@ -206,10 +205,10 @@
 
   I.Access = AccessSpecifier::AS_none;
 
-  I.ReturnType =
-  TypeInfo(EmptySID, "void", InfoType::IT_default, "path/to/void");
-  I.Params.emplace_back("int", "path/to/int", "P");
-  I.Params.emplace_back("double", "path/to/double", "D");
+  I.ReturnType = TypeInfo(
+  Reference(EmptySID, "void", InfoType::IT_default, "path/to/void"));
+  I.Params.emplace_back(TypeInfo("int", "path/to/int"), "P");
+  I.Params.emplace_back(TypeInfo("double", "path/to/double"), "D");
   I.Params.back().DefaultValue = "2.0 * M_PI";
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
@@ -335,9 +334,9 @@
   FunctionInfo I;
   I.Name = "f";
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
-  I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
-  I.Params.emplace_back("int", "I");
-  I.Params.emplace_back("int", "J");
+  I.ReturnType = TypeInfo("void");
+  I.Params.emplace_back(TypeInfo("int"), "I");
+  I.Params.emplace_back(TypeInfo("int"), "J");
   I.Access = AccessSpecifier::AS_none;
 
   CommentInfo Top;
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -119,7 +119,7 @@
   NamespaceInfo ExpectedBWithFunction(EmptySID);
   FunctionInfo F;
   F.Name = "f";
-  F.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  F.ReturnType = TypeInfo("void");
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   F.Namespace.emplace_back(EmptySID, "B", InfoType::IT_namespace);
   F.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
@@ -165,7 +165,8 @@
InfoType::IT_namespace);
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
-  ExpectedE.Members.emplace_back("int", "value", AccessSpecifier::AS_public);
+  ExpectedE.Members.emplace_back(TypeInfo("int"), "value",
+ AccessSpecifier::AS_public);
   // TODO the data member should have the docstring on it:
   //ExpectedE.Members.back().Description.push_back(MakeOneLineCommentInfo(" Some docs"));
   CheckRecordInfo(&ExpectedE, E);
@@ -175,7 +176,7 @@
   FunctionInfo EConstructor;
   EConstructor.Name = "E";
   EConstructor.Parent = Reference(EmptySID, "E", InfoType::IT_record);
-  EConstructor.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  EConstructor.ReturnType = TypeInfo("void");
   EConstructor.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   EConstructor.Namespace.emplace_back(EmptySID, "E", I

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 462313.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134371/new/

https://reviews.llvm.org/D134371

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,15 +28,16 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace, "path/to/A/Namespace");
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/Namespace");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace,
+ "path/to/A/Namespace");
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Functions.back().Access = AccessSpecifier::AS_none;
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -100,8 +101,8 @@
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
-  I.Bases.back().ChildFunctions.emplace_back();
-  I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
+  I.Bases.back().Children.Functions.emplace_back();
+  I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
   I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
@@ -109,12 +110,12 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -330,6 +331,30 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(YAMLGeneratorTest, enumTypedefYAML) {
+  TypedefInfo I;
+  I.Name = "MyUsing";
+  I.Underlying = TypeInfo("int");
+  I.IsUsing = true;
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'MyUsing'
+Underlying:
+  Name:'int'
+IsUsing: true
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 TEST(YAMLGeneratorTest, emitCommentYAML) {
   FunctionInfo I;
   I.Name = "f";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clan

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:540
 }
-
 template <> void addChild(NamespaceInfo *I, EnumInfo &&R) {

paulkirth wrote:
> nit: can we avoid unrelated changes to  whitespace here and elsewhere in the 
> patch?
I did this on purpose. There were 8 variants of AddChild and I added two more. 
It became difficult to find the right one so I grouped them with comments for 
each group. This may be difficult to see in the side-by-side but it's much 
nicer in an editor.



Comment at: clang-tools-extra/clang-doc/Representation.h:280
 // Info for namespaces.
-struct NamespaceInfo : public Info {
+struct NamespaceInfo : public Info, public ScopeHasChildren {
   NamespaceInfo(SymbolID USR = SymbolID(), StringRef Name = StringRef(),

paulkirth wrote:
> It seems a bit odd to use inheritance here on a type w/ public fields, and no 
> methods. do you think using composition would improve the situation?
> 
>  I'm fine w/ it if we think this is the simpler/more maintainable solution, 
> but given that we don't have any methods in this case I'm unsure if there's 
> much benefit.
I don't have a strong opinion. I changed to composition.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:633
+  // both the parent and the record itself.
+  return {std::move(I), MakeAndInsertIntoParent(*I)};
 }

paulkirth wrote:
> Won't this deref the moved from `unique_ptr`? 
> 
> https://en.cppreference.com/w/cpp/language/eval_order
> > In list-initialization, every value computation and side effect of a given 
> > initializer clause is sequenced before every value computation and side 
> > effect associated with any initializer clause that follows it in the 
> > brace-enclosed comma-separated list of initializers.
> 
> Under that reading, the move //should// always happen first. If it happens to 
> still work, I think it's just luck in the implementation.
Thanks, nice catch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134371/new/

https://reviews.llvm.org/D134371

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.h:202
+   bool IsFileInRootDir = false)
   : LineNumber(LineNumber), Filename(std::move(Filename)),
 IsFileInRootDir(IsFileInRootDir) {}

paulkirth wrote:
> do we still want to use `std::move` here now that its a StringRef? I don't 
> think we would do that w/ `const char*`, which is what `StringRef` boils down 
> to. It should be passed by value everywhere.
> 
> see 
> https://llvm.org/docs/ProgrammersManual.html#passing-strings-the-stringref-and-twine-classes,
>  
> https://llvm.org/docs/ProgrammersManual.html#the-stringref-class 
> https://llvm.org/docs/ProgrammersManual.html#string-like-containers
The move is unnecessary and I removed it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134235/new/

https://reviews.llvm.org/D134235

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.

I can guarantee that nobody uses this for anything useful because it doesn't 
work well enough. For example, before I started changing it, requesting HTML 
output crashed on startup and many of the most basic syntactic things were 
broken. My previous patch changed enums from being a list of dictionaries which 
is much more backwards incompatible than this. I don't think we need to worry 
about that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134235/new/

https://reviews.llvm.org/D134235

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134235: [clang-doc] Clean up *Info constructors.

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:336
 Reference *I, FieldId &F) {
   switch (ID) {
   case REFERENCE_USR:

haowei wrote:
> This block ID came from L582, which read from a bitcode file when 
> `BI_REFERENCE_BLOCK_ID` is encountered. I am not familiar with the content of 
> an arbitrary bitcode file. But after your patch, if this block ID is 
> encountered, it will result in an error (a behavior change).
> 
> It looks like `REFERENCE_IS_IN_GLOBAL_NAMESPACE` is only defined in in 
> clang-doc and not part of LLVM's bitcode header. So it looks like it is not a 
> standardized ID to me. Could you confirm it?
> 
> If `REFERENCE_IS_IN_GLOBAL_NAMESPACE` only presents in bitcode file generated 
> from BitcodeWriter from clang-doc and we are not expecting to provide 
> compatibility to an arbitrary bitcode generated from elsewhere, I am OK with 
> deleting this field (I still don't think this is the right approach to do but 
> I don't have a strong opinion on this either). 
Correct, this whole bitcode system is used only to communicate within clang-doc 
between threads (!) (I think it should be entirely removed but I don't have the 
time for that). It is never serialized to a file and is not shared with any 
other component.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134235/new/

https://reviews.llvm.org/D134235

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 466858.
brettw added a comment.

Fixed -Wall issues.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134371/new/

https://reviews.llvm.org/D134371

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,15 +28,16 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace, "path/to/A/Namespace");
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/Namespace");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace,
+ "path/to/A/Namespace");
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Functions.back().Access = AccessSpecifier::AS_none;
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -100,8 +101,8 @@
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
-  I.Bases.back().ChildFunctions.emplace_back();
-  I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
+  I.Bases.back().Children.Functions.emplace_back();
+  I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
   I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
@@ -109,12 +110,12 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -330,6 +331,30 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(YAMLGeneratorTest, enumTypedefYAML) {
+  TypedefInfo I;
+  I.Name = "MyUsing";
+  I.Underlying = TypeInfo("int");
+  I.IsUsing = true;
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'MyUsing'
+Underlying:
+  Name:'int'
+IsUsing: true
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 TEST(YAMLGeneratorTest, emitCommentYAML) {
   FunctionInfo I;
   I.Name = "f";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/Serializ

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw reopened this revision.
brettw added a comment.
This revision is now accepted and ready to land.

New patch up to fix warnings that caused the revert.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134371/new/

https://reviews.llvm.org/D134371

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 467894.
brettw added a comment.

Rebase


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134371/new/

https://reviews.llvm.org/D134371

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,15 +28,16 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace, "path/to/A/Namespace");
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/Namespace");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace,
+ "path/to/A/Namespace");
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Functions.back().Access = AccessSpecifier::AS_none;
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -100,8 +101,8 @@
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
-  I.Bases.back().ChildFunctions.emplace_back();
-  I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
+  I.Bases.back().Children.Functions.emplace_back();
+  I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
   I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
@@ -109,12 +110,12 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -330,6 +331,30 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(YAMLGeneratorTest, enumTypedefYAML) {
+  TypedefInfo I;
+  I.Name = "MyUsing";
+  I.Underlying = TypeInfo("int");
+  I.IsUsing = true;
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'MyUsing'
+Underlying:
+  Name:'int'
+IsUsing: true
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 TEST(YAMLGeneratorTest, emitCommentYAML) {
   FunctionInfo I;
   I.Name = "f";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 467895.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134371/new/

https://reviews.llvm.org/D134371

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,15 +28,16 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace, "path/to/A/Namespace");
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/Namespace");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace,
+ "path/to/A/Namespace");
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Functions.back().Access = AccessSpecifier::AS_none;
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -100,8 +101,8 @@
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
-  I.Bases.back().ChildFunctions.emplace_back();
-  I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
+  I.Bases.back().Children.Functions.emplace_back();
+  I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
   I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
@@ -109,12 +110,12 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -330,6 +331,30 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(YAMLGeneratorTest, enumTypedefYAML) {
+  TypedefInfo I;
+  I.Name = "MyUsing";
+  I.Underlying = TypeInfo("int");
+  I.IsUsing = true;
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'MyUsing'
+Underlying:
+  Name:'int'
+IsUsing: true
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 TEST(YAMLGeneratorTest, emitCommentYAML) {
   FunctionInfo I;
   I.Name = "f";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clan

[PATCH] D134371: [clang-doc] Add typedef/using information.

[Brett Wilson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG21fb70c6ab3b: [clang-doc] Add typedef/using information. 
(authored by brettw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134371/new/

https://reviews.llvm.org/D134371

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,15 +28,16 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace, "path/to/A/Namespace");
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/Namespace");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
+ InfoType::IT_namespace,
+ "path/to/A/Namespace");
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/Namespace");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Functions.back().Access = AccessSpecifier::AS_none;
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -100,8 +101,8 @@
   I.TagType = TagTypeKind::TTK_Class;
   I.Bases.emplace_back(EmptySID, "F", "path/to/F", true,
AccessSpecifier::AS_public, true);
-  I.Bases.back().ChildFunctions.emplace_back();
-  I.Bases.back().ChildFunctions.back().Name = "InheritedFunctionOne";
+  I.Bases.back().Children.Functions.emplace_back();
+  I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
   I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
@@ -109,12 +110,12 @@
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
-  I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
-  I.ChildFunctions.emplace_back();
-  I.ChildFunctions.back().Name = "OneFunction";
-  I.ChildEnums.emplace_back();
-  I.ChildEnums.back().Name = "OneEnum";
+  I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path/to/A/r");
+  I.Children.Functions.emplace_back();
+  I.Children.Functions.back().Name = "OneFunction";
+  I.Children.Enums.emplace_back();
+  I.Children.Enums.back().Name = "OneEnum";
 
   auto G = getYAMLGenerator();
   assert(G);
@@ -330,6 +331,30 @@
   EXPECT_EQ(Expected, Actual.str());
 }
 
+TEST(YAMLGeneratorTest, enumTypedefYAML) {
+  TypedefInfo I;
+  I.Name = "MyUsing";
+  I.Underlying = TypeInfo("int");
+  I.IsUsing = true;
+
+  auto G = getYAMLGenerator();
+  assert(G);
+  std::string Buffer;
+  llvm::raw_string_ostream Actual(Buffer);
+  auto Err = G->generateDocForInfo(&I, Actual, ClangDocContext());
+  assert(!Err);
+  std::string Expected =
+  R"raw(---
+USR: ''
+Name:'MyUsing'
+Underlying:
+  Name:'int'
+IsUsing: true
+...
+)raw";
+  EXPECT_EQ(Expected, Actual.str());
+}
+
 TEST(YAMLGeneratorTest, emitCommentYAML) {
   FunctionInfo I;
   I.Name = "f";
Index: clang-tools-extra/unittests/clang-doc/SerializeTest.c

[PATCH] D136894: Add clang-doc readme

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.

I definitely wasn't intending to write "formal documentation" for clang-doc. I 
was trying to write some notes for a future contributor as I'm winding down my 
work here, and it seems nobody else is working on it or has any knowledge about 
it. Is there a place in LLVM for this type of documentation? I can rename the 
file to "notes" or something if you think that would help make things more 
clear.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136894/new/

https://reviews.llvm.org/D136894

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136894: Add clang-doc readme

[Brett Wilson via Phabricator via cfe-commits]

brettw abandoned this revision.
brettw added a comment.

Ok thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136894/new/

https://reviews.llvm.org/D136894

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.

Previously file naming and directory layout was handled on a per Info object 
basis by ClangDocMain and the generators blindly wrote to the files given. This 
means all generators must use the same file layout and caused problems where 
multiple objects mapped to the same file. The object collision problem happens 
most easily with template specializations because the template parameters are 
not part of the "name".

  

This patch moves the responsibility for output file organization to the 
generators. Currently HTML and MD use the same structure as before. But they 
now collect all objects that map to a given file and combine them, avoiding the 
corruption problems.

  

Converts the YAML generator to naming files based on USR in one directory. This 
is easier for downstream tools to manage and avoids the naming problems with 
template specializations. Since this change requires backward-incompatible 
output changes to referenced files anyway (since each one is now an array), 
this is a good time to introduce this change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -130,54 +131,6 @@
   return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
 }
 
-bool CreateDirectory(const Twine &DirName, bool ClearDirectory = false) {
-  std::error_code OK;
-  llvm::SmallString<128> DocsRootPath;
-  if (ClearDirectory) {
-std::error_code RemoveStatus = llvm::sys::fs::remove_directories(DirName);
-if (RemoveStatus != OK) {
-  llvm::errs() << "Unable to remove existing documentation directory for "
-   << DirName << ".\n";
-  return true;
-}
-  }
-  std::error_code DirectoryStatus = llvm::sys::fs::create_directories(DirName);
-  if (DirectoryStatus != OK) {
-llvm::errs() << "Unable to create documentation directories.\n";
-return true;
-  }
-  return false;
-}
-
-// A function to extract the appropriate file name for a given info's
-// documentation. The path returned is a composite of the output directory, the
-// info's relative path and name and the extension. The relative path should
-// have been constructed in the serialization phase.
-//
-// Example: Given the below, the  path for class C will be
-// /A/B/C.
-//
-// namespace A {
-// namespace B {
-//
-// class C {};
-//
-// }
-// }
-llvm::Expected> getInfoOutputFile(StringRef Root,
- StringRef RelativePath,
- StringRef Name,
- StringRef Ext) {
-  llvm::SmallString<128> Path;
-  llvm::sys::path::native(Root, Path);
-  llvm::sys::path::append(Path, RelativePath);
-  if (CreateDirectory(Path))
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "failed to create directory");
-  llvm::sys::path::append(Path, Name + Ext);
-  return Path;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -274,6 +227,11 @@
 R.first->second.emplace_back(Value);
   });
 
+  // Collects all Infos according to their unique USR value. This map is added
+  // to from the thread pool below and is protected by the USRToInfoMutex.
+  llvm::sys::Mutex USRToInfoMutex;
+  llvm::StringMap> USRToInfo;
+
   // First reducing phase (reduce all decls into one info per decl).
   llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
   std::atomic Error;
@@ -304,31 +262,17 @@
 return;
   }
 
-  doc::Info *I = Reduced.get().get();
-  auto InfoPath =
-  getInfoOutputFile(OutDirectory, I->getRelativeFilePath(""),
-I->getFileBaseName(), "." + Format);
-  if (!InfoPath) {
-llvm::errs() << toString(InfoPath.takeError()) << "\n";
-Error = true;
-return;
-  }
-  std::error_code FileErr;
-  llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
-  llvm::sys::fs::OF_None);
-  

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw added a comment.

If you're curious, you can see the simplification the YAML output format change 
makes in the consuming code:
https://fuchsia-review.git.corp.google.com/c/fuchsia/+/760570/2/tools/cppdocgen/clangdoc/clangdoc.go
Since it allows us to remove some special-casing code for loading things in the 
global namespace and for anonymous records.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 475869.
brettw added a comment.

Updated the lit tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/`ls %t/docs | grep -v index.yaml` | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -130,54 +131,6 @@
   return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
 }
 
-bool CreateDirectory(const Twine &DirName, bool ClearDirectory = false) {
-  std::error_code OK;
-  llvm::SmallString<128> DocsRootPath;
-  if (ClearDirectory) {
-std::error_code RemoveStatus = llvm::sys::fs::remove_directories(DirName);
-if (RemoveStatus != OK) {
-  llvm::errs() << "Unable to remove existing documentation directory for "
-   << DirName << ".\n";
-  return true;
-}
-  }
-  std::error_code DirectoryStatus = llvm::sys::fs::create_directories(DirName);
-  if (DirectoryStatus != OK) {
-llvm::errs() << "Unable to create documentation directories.\n";
-return true;
-  }
-  return false;
-}
-
-// A function to extract the appropriate file name for a given info's
-// documentation. The path returned is a composite of the output directory, the
-// info's relative path and name and the extension. The relative path should
-// have been constructed in the serialization phase.
-//
-// Example: Given the below, the  path for class C will be
-// /A/B/C.
-//
-// namespace A {
-// namespace B {
-//
-// class C {};
-//
-// }
-// }
-llvm::Expected> getInfoOutputFile(StringRef Root,
- StringRef RelativePath,
- StringRef Name,
- StringRef Ext) {
-  llvm::SmallString<128> Path;
-  llvm::sys::path::native(Root, Path);
-  llvm::sys::path::append(Path, RelativePath);
-  if (CreateDirectory(Path))
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "failed to create directory");
-  llvm::sys::path::append(Path, Name + Ext);
-  return Path;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -274,6 +227,11 @@
 R.first->second.emplace_back(Value);
   });
 
+  // Collects all Infos according to their unique USR value. This map is added
+  // to from the thread pool below and is protected by the USRToInfoMutex.
+  llvm::sys::Mutex USRToInfoMutex;
+  llvm::StringMap> USRToInfo;
+
   // First reducing phase (reduce all decls into one info per decl).
   llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
   std::atomic Error;
@@ -304,31 +262,17 @@
 return;
   }
 
-  doc::Info *I = Reduced.get().get();
-  auto InfoPath =
-  getInfoOutputFile(OutDirectory, I->getRelativeFilePath(""),
-I->getFileBaseName(), "." + Format);
-  if (!InfoPath) {
-llvm::errs() << toString(InfoPath.takeError()) << "\n";
-Error = true;
-return;
-  }
-  std::erro

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 476183.
brettw marked 3 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,9 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+//   This produces two files, index.yaml and one for the record named by its USR
+//   (which we don't know in advance). This checks the record file.
+// RUN: ls %t/docs | grep -v index.yaml | xargs -IYAML_RECORD_FILE cat %t/docs/YAML_RECORD_FILE | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -130,54 +131,6 @@
   return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
 }
 
-bool CreateDirectory(const Twine &DirName, bool ClearDirectory = false) {
-  std::error_code OK;
-  llvm::SmallString<128> DocsRootPath;
-  if (ClearDirectory) {
-std::error_code RemoveStatus = llvm::sys::fs::remove_directories(DirName);
-if (RemoveStatus != OK) {
-  llvm::errs() << "Unable to remove existing documentation directory for "
-   << DirName << ".\n";
-  return true;
-}
-  }
-  std::error_code DirectoryStatus = llvm::sys::fs::create_directories(DirName);
-  if (DirectoryStatus != OK) {
-llvm::errs() << "Unable to create documentation directories.\n";
-return true;
-  }
-  return false;
-}
-
-// A function to extract the appropriate file name for a given info's
-// documentation. The path returned is a composite of the output directory, the
-// info's relative path and name and the extension. The relative path should
-// have been constructed in the serialization phase.
-//
-// Example: Given the below, the  path for class C will be
-// /A/B/C.
-//
-// namespace A {
-// namespace B {
-//
-// class C {};
-//
-// }
-// }
-llvm::Expected> getInfoOutputFile(StringRef Root,
- StringRef RelativePath,
- StringRef Name,
- StringRef Ext) {
-  llvm::SmallString<128> Path;
-  llvm::sys::path::native(Root, Path);
-  llvm::sys::path::append(Path, RelativePath);
-  if (CreateDirectory(Path))
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "failed to create directory");
-  llvm::sys::path::append(Path, Name + Ext);
-  return Path;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -274,6 +227,11 @@
 R.first->second.emplace_back(Value);
   });
 
+  // Collects all Infos according to their unique USR value. This map is added
+  // to from the thread pool below and is protected by the USRToInfoMutex.
+  llvm::sys::Mutex USRToInfoMutex;
+  llvm::StringMap> USRToInfo;
+
   // First reducing phase (reduce all decls into one info per decl).
   llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
   std::atomic Error;
@@ -304,31 +262,17 @@
 return;
   }
 
-  doc::Info *I = Reduced.get().get();
-  auto InfoPath =
-  getInfoOutputFile(OutDirectory, I->getRelativeFilePath(""),
-I->get

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:890-892
+// TODO If there are multiple Infos for this file name (for example,
+// template specializations), this will generate multiple complete web 
pages
+// (with  and , etc.) concatenated together. This generator

paulkirth wrote:
> So does this patch make HTML unusable? Currently, if there is a conflict, the 
> file gets replaced, right? 
> 
> I read this as we're going from a situation where we lost data, but still had 
> valid HTML pages to one where we have complete, but incorrect HTML, is that 
> correct?
> 
> Also, why does that happen under template specialization? USRs are SHA1 
> hashes of the symbol. I would expect that hashing the symbol would be 
> unambiguous and unique, given C++'s ODR. 
> 
> That last point made me wonder if an alternate way to handle this without 
> using the hash would be to use the mangled names?
I think this makes HTML strictly better (though not ideal).

Previously if there was a collision, they got written over the top of each 
other without truncation (not sure why). So if the longer page was written 
second, you got lucky and you get a complete valid page and just lost the 
shorter struct's definition. If you got unlucky the shorter page was written 
second and you get a corrupted file with the shorter content, followed by the 
end of the longer content peeking out from the end.

With this change, you get both content concatenated without corruption, you 
just have duplicate doctype and title tags in between them. Browsers should 
generally handle this so the page should be usable.

Fixing this requires a refactor of the HTML generator such that the concept of 
writing a struct is separate from writing a page. Even if I was going to work 
on the HTML generator, I would probably want to do this in a separate patch 
anyway.

The markdown generator does the same, but markdown doesn't have all of the 
headers stuff that shouldn't be in the middle of the file, you just get a new 
`# ...` for the second section and it should be reasonable.

The HTML and MD generators name files according to the `Name` of the structure. 
USR is not involved. So any time to things in the same namespace have the same 
name, you'll get a collision. This happens most commonly with template 
specialization because the names are identical (the name doesn't account for 
the template parameters).

All of this complexity is why I changed the YAML output to use USR which avoids 
the problem. I didn't want to make this change for the HTML and MD generators 
because it will make ugly file names that the user will see, and you probably 
want the template specializations to be in the same file anyway.



Comment at: clang-tools-extra/test/clang-doc/single-file-public.cpp:6
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp 
-output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s 
--check-prefix=CHECK
+// RUN: cat %t/docs/`ls %t/docs | grep -v index.yaml` | FileCheck %s 
--check-prefix=CHECK
 // RUN: rm -rf %t

paulkirth wrote:
> Its OK to use `ls` and `grep` in the runline, but you cannot use the backtick 
> syntax. It will cause problems on other platforms and I can't find any other 
> usage of that in our codebase. TBH I'm a bit surprised this works, since 
> `lit` has some interesting behavior around RUN. 
> 
> I think you can just use `cat < ls %t/docs | grep -v index.yaml` instead. I 
> haven't tried it, but that is certainly a more typical method of doing this 
> in a lit test. Using `find` may also work, but I only found one use in 
> `llvm/test/ExecutionEngine/MCJIT/load-object-a.ll`.
Your example doesn't work (it's trying to load a file named "ls") but I found 
another solution using xargs (which is hopefully supported on all of the 
platforms).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 476612.
brettw marked an inline comment as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,10 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+//   This produces two files, index.yaml and one for the record named by its USR
+//   (which we don't know in advance). This checks the record file by searching
+//   for a name with a 40-char USR name.
+// RUN: find %t -regex ".*[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z].*" -exec cat {} \; | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -130,54 +131,6 @@
   return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
 }
 
-bool CreateDirectory(const Twine &DirName, bool ClearDirectory = false) {
-  std::error_code OK;
-  llvm::SmallString<128> DocsRootPath;
-  if (ClearDirectory) {
-std::error_code RemoveStatus = llvm::sys::fs::remove_directories(DirName);
-if (RemoveStatus != OK) {
-  llvm::errs() << "Unable to remove existing documentation directory for "
-   << DirName << ".\n";
-  return true;
-}
-  }
-  std::error_code DirectoryStatus = llvm::sys::fs::create_directories(DirName);
-  if (DirectoryStatus != OK) {
-llvm::errs() << "Unable to create documentation directories.\n";
-return true;
-  }
-  return false;
-}
-
-// A function to extract the appropriate file name for a given info's
-// documentation. The path returned is a composite of the output directory, the
-// info's relative path and name and the extension. The relative path should
-// have been constructed in the serialization phase.
-//
-// Example: Given the below, the  path for class C will be
-// /A/B/C.
-//
-// namespace A {
-// namespace B {
-//
-// class C {};
-//
-// }
-// }
-llvm::Expected> getInfoOutputFile(StringRef Root,
- StringRef RelativePath,
- StringRef Name,
- StringRef Ext) {
-  llvm::SmallString<128> Path;
-  llvm::sys::path::native(Root, Path);
-  llvm::sys::path::append(Path, RelativePath);
-  if (CreateDirectory(Path))
-return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "failed to create directory");
-  llvm::sys::path::append(Path, Name + Ext);
-  return Path;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -274,6 +227,11 @@
 R.first->second.emplace_back(Value);
   });
 
+  // Collects all Infos according to their unique USR value. This map is added
+  // to from the thread pool below and is protected by the USRToInfoMutex.
+  llvm::sys::Mutex USRToInfoMutex;
+  llvm::StringMap> USRToInfo;
+
   // First reducing phase (reduce all decls into one info pe

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:890-892
+// TODO If there are multiple Infos for this file name (for example,
+// template specializations), this will generate multiple complete web 
pages
+// (with  and , etc.) concatenated together. This generator

paulkirth wrote:
> brettw wrote:
> > paulkirth wrote:
> > > So does this patch make HTML unusable? Currently, if there is a conflict, 
> > > the file gets replaced, right? 
> > > 
> > > I read this as we're going from a situation where we lost data, but still 
> > > had valid HTML pages to one where we have complete, but incorrect HTML, 
> > > is that correct?
> > > 
> > > Also, why does that happen under template specialization? USRs are SHA1 
> > > hashes of the symbol. I would expect that hashing the symbol would be 
> > > unambiguous and unique, given C++'s ODR. 
> > > 
> > > That last point made me wonder if an alternate way to handle this without 
> > > using the hash would be to use the mangled names?
> > I think this makes HTML strictly better (though not ideal).
> > 
> > Previously if there was a collision, they got written over the top of each 
> > other without truncation (not sure why). So if the longer page was written 
> > second, you got lucky and you get a complete valid page and just lost the 
> > shorter struct's definition. If you got unlucky the shorter page was 
> > written second and you get a corrupted file with the shorter content, 
> > followed by the end of the longer content peeking out from the end.
> > 
> > With this change, you get both content concatenated without corruption, you 
> > just have duplicate doctype and title tags in between them. Browsers should 
> > generally handle this so the page should be usable.
> > 
> > Fixing this requires a refactor of the HTML generator such that the concept 
> > of writing a struct is separate from writing a page. Even if I was going to 
> > work on the HTML generator, I would probably want to do this in a separate 
> > patch anyway.
> > 
> > The markdown generator does the same, but markdown doesn't have all of the 
> > headers stuff that shouldn't be in the middle of the file, you just get a 
> > new `# ...` for the second section and it should be reasonable.
> > 
> > The HTML and MD generators name files according to the `Name` of the 
> > structure. USR is not involved. So any time to things in the same namespace 
> > have the same name, you'll get a collision. This happens most commonly with 
> > template specialization because the names are identical (the name doesn't 
> > account for the template parameters).
> > 
> > All of this complexity is why I changed the YAML output to use USR which 
> > avoids the problem. I didn't want to make this change for the HTML and MD 
> > generators because it will make ugly file names that the user will see, and 
> > you probably want the template specializations to be in the same file 
> > anyway.
> > I think this makes HTML strictly better (though not ideal).
> > 
> > Previously if there was a collision, they got written over the top of each 
> > other without truncation (not sure why). So if the longer page was written 
> > second, you got lucky and you get a complete valid page and just lost the 
> > shorter struct's definition. If you got unlucky the shorter page was 
> > written second and you get a corrupted file with the shorter content, 
> > followed by the end of the longer content peeking out from the end.
> > 
> > With this change, you get both content concatenated without corruption, you 
> > just have duplicate doctype and title tags in between them. Browsers should 
> > generally handle this so the page should be usable.
> > 
> 
> Thanks for the explanation. My concern was that this change made HTML 
> unusable, if it is still valid (if not ideal) then I don't think there is an 
> issue.
> 
> > Fixing this requires a refactor of the HTML generator such that the concept 
> > of writing a struct is separate from writing a page. Even if I was going to 
> > work on the HTML generator, I would probably want to do this in a separate 
> > patch anyway.
> > 
> 
> No arguments re splitting up work. Can you file an issue to track this on 
> github and reference it in the TODO? That should help make sure this gets 
> addressed.
> 
> > The markdown generator does the same, but markdown doesn't have all of the 
> > headers stuff that shouldn't be in the middle of the file, you just get a 
> > new `# ...` for the second section and it should be reasonable.
> > 
> > The HTML and MD generators name files according to the `Name` of the 
> > structure. USR is not involved. So any time to things in the same namespace 
> > have the same name, you'll get a collision. This happens most commonly with 
> > template specialization because the names are identical (the name doesn't 
> > account for the template parameters).
> > 
> 
> I think there should be 

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 476955.
brettw marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,10 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+//   This produces two files, index.yaml and one for the record named by its USR
+//   (which we don't know in advance). This checks the record file by searching
+//   for a name with a 40-char USR name.
+// RUN: find %t -regextype sed -regex ".*[0-9A-F]\{40\}.yaml" -exec cat {} ";" | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
@@ -19,7 +22,7 @@
 void Record::function_public() {}
 
 // CHECK: ---
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'Record'
 // CHECK-NEXT: Path:'GlobalNamespace'
 // CHECK-NEXT: Namespace:
@@ -30,12 +33,12 @@
 // CHECK-NEXT:   Filename:'{{.*}}'
 // CHECK-NEXT: TagType: Class
 // CHECK-NEXT: ChildFunctions:
-// CHECK-NEXT:   - USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT:   - USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'function_public'
 // CHECK-NEXT: Namespace:
 // CHECK-NEXT:   - Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT:   - Type:Namespace
 // CHECK-NEXT: Name:'GlobalNamespace'
 // CHECK-NEXT: DefLocation:
@@ -48,7 +51,7 @@
 // CHECK-NEXT: Parent:
 // CHECK-NEXT: Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: ReturnType:
 // CHECK-NEXT:   Type:
 // CHECK-NEXT: Name:'void'
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#inc

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:890-892
+// TODO If there are multiple Infos for this file name (for example,
+// template specializations), this will generate multiple complete web 
pages
+// (with  and , etc.) concatenated together. This generator

paulkirth wrote:
> brettw wrote:
> > paulkirth wrote:
> > > brettw wrote:
> > > > paulkirth wrote:
> > > > > So does this patch make HTML unusable? Currently, if there is a 
> > > > > conflict, the file gets replaced, right? 
> > > > > 
> > > > > I read this as we're going from a situation where we lost data, but 
> > > > > still had valid HTML pages to one where we have complete, but 
> > > > > incorrect HTML, is that correct?
> > > > > 
> > > > > Also, why does that happen under template specialization? USRs are 
> > > > > SHA1 hashes of the symbol. I would expect that hashing the symbol 
> > > > > would be unambiguous and unique, given C++'s ODR. 
> > > > > 
> > > > > That last point made me wonder if an alternate way to handle this 
> > > > > without using the hash would be to use the mangled names?
> > > > I think this makes HTML strictly better (though not ideal).
> > > > 
> > > > Previously if there was a collision, they got written over the top of 
> > > > each other without truncation (not sure why). So if the longer page was 
> > > > written second, you got lucky and you get a complete valid page and 
> > > > just lost the shorter struct's definition. If you got unlucky the 
> > > > shorter page was written second and you get a corrupted file with the 
> > > > shorter content, followed by the end of the longer content peeking out 
> > > > from the end.
> > > > 
> > > > With this change, you get both content concatenated without corruption, 
> > > > you just have duplicate doctype and title tags in between them. 
> > > > Browsers should generally handle this so the page should be usable.
> > > > 
> > > > Fixing this requires a refactor of the HTML generator such that the 
> > > > concept of writing a struct is separate from writing a page. Even if I 
> > > > was going to work on the HTML generator, I would probably want to do 
> > > > this in a separate patch anyway.
> > > > 
> > > > The markdown generator does the same, but markdown doesn't have all of 
> > > > the headers stuff that shouldn't be in the middle of the file, you just 
> > > > get a new `# ...` for the second section and it should be reasonable.
> > > > 
> > > > The HTML and MD generators name files according to the `Name` of the 
> > > > structure. USR is not involved. So any time to things in the same 
> > > > namespace have the same name, you'll get a collision. This happens most 
> > > > commonly with template specialization because the names are identical 
> > > > (the name doesn't account for the template parameters).
> > > > 
> > > > All of this complexity is why I changed the YAML output to use USR 
> > > > which avoids the problem. I didn't want to make this change for the 
> > > > HTML and MD generators because it will make ugly file names that the 
> > > > user will see, and you probably want the template specializations to be 
> > > > in the same file anyway.
> > > > I think this makes HTML strictly better (though not ideal).
> > > > 
> > > > Previously if there was a collision, they got written over the top of 
> > > > each other without truncation (not sure why). So if the longer page was 
> > > > written second, you got lucky and you get a complete valid page and 
> > > > just lost the shorter struct's definition. If you got unlucky the 
> > > > shorter page was written second and you get a corrupted file with the 
> > > > shorter content, followed by the end of the longer content peeking out 
> > > > from the end.
> > > > 
> > > > With this change, you get both content concatenated without corruption, 
> > > > you just have duplicate doctype and title tags in between them. 
> > > > Browsers should generally handle this so the page should be usable.
> > > > 
> > > 
> > > Thanks for the explanation. My concern was that this change made HTML 
> > > unusable, if it is still valid (if not ideal) then I don't think there is 
> > > an issue.
> > > 
> > > > Fixing this requires a refactor of the HTML generator such that the 
> > > > concept of writing a struct is separate from writing a page. Even if I 
> > > > was going to work on the HTML generator, I would probably want to do 
> > > > this in a separate patch anyway.
> > > > 
> > > 
> > > No arguments re splitting up work. Can you file an issue to track this on 
> > > github and reference it in the TODO? That should help make sure this gets 
> > > addressed.
> > > 
> > > > The markdown generator does the same, but markdown doesn't have all of 
> > > > the headers stuff that shouldn't be in the middle of the file, you just 
> > > > get a new `# ...` for the second section and it should be reasonable.
> > > > 
> > > > The HTML

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8a469fc5727: [clang-doc] Move file layout to the 
generators. (authored by brettw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,10 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+//   This produces two files, index.yaml and one for the record named by its USR
+//   (which we don't know in advance). This checks the record file by searching
+//   for a name with a 40-char USR name.
+// RUN: find %t -regextype sed -regex ".*[0-9A-F]\{40\}.yaml" -exec cat {} ";" | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
@@ -19,7 +22,7 @@
 void Record::function_public() {}
 
 // CHECK: ---
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'Record'
 // CHECK-NEXT: Path:'GlobalNamespace'
 // CHECK-NEXT: Namespace:
@@ -30,12 +33,12 @@
 // CHECK-NEXT:   Filename:'{{.*}}'
 // CHECK-NEXT: TagType: Class
 // CHECK-NEXT: ChildFunctions:
-// CHECK-NEXT:   - USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT:   - USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'function_public'
 // CHECK-NEXT: Namespace:
 // CHECK-NEXT:   - Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT:   - Type:Namespace
 // CHECK-NEXT: Name:'GlobalNamespace'
 // CHECK-NEXT: DefLocation:
@@ -48,7 +51,7 @@
 // CHECK-NEXT: Parent:
 // CHECK-NEXT: Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: ReturnType:
 // CHECK-NEXT:   Type:
 // CHECK-NEXT: Name:'void'
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 478683.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,10 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+//   This produces two files, index.yaml and one for the record named by its USR
+//   (which we don't know in advance). This checks the record file by searching
+//   for a name with a 40-char USR name.
+// RUN: find %t/docs -regex ".*/[0-9A-F]*.yaml" -exec cat {} ";" | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
@@ -19,7 +22,7 @@
 void Record::function_public() {}
 
 // CHECK: ---
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'Record'
 // CHECK-NEXT: Path:'GlobalNamespace'
 // CHECK-NEXT: Namespace:
@@ -30,12 +33,12 @@
 // CHECK-NEXT:   Filename:'{{.*}}'
 // CHECK-NEXT: TagType: Class
 // CHECK-NEXT: ChildFunctions:
-// CHECK-NEXT:   - USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT:   - USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'function_public'
 // CHECK-NEXT: Namespace:
 // CHECK-NEXT:   - Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT:   - Type:Namespace
 // CHECK-NEXT: Name:'GlobalNamespace'
 // CHECK-NEXT: DefLocation:
@@ -48,7 +51,7 @@
 // CHECK-NEXT: Parent:
 // CHECK-NEXT: Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: ReturnType:
 // CHECK-NEXT:   Type:
 // CHECK-NEXT: Name:'void'
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -130,54 +131,6 @@
   return llvm::

[PATCH] D138073: [clang-doc] Move file layout to the generators.

[Brett Wilson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b8c7e02122a: [clang-doc] Move file layout to the 
generators. (authored by brettw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138073/new/

https://reviews.llvm.org/D138073

Files:
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp

Index: clang-tools-extra/test/clang-doc/single-file.cpp
===
--- clang-tools-extra/test/clang-doc/single-file.cpp
+++ clang-tools-extra/test/clang-doc/single-file.cpp
@@ -3,7 +3,7 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);
Index: clang-tools-extra/test/clang-doc/single-file-public.cpp
===
--- clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,7 +3,10 @@
 // RUN: echo "" > %t/compile_flags.txt
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs
-// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
+//   This produces two files, index.yaml and one for the record named by its USR
+//   (which we don't know in advance). This checks the record file by searching
+//   for a name with a 40-char USR name.
+// RUN: find %t/docs -regex ".*/[0-9A-F]*.yaml" -exec cat {} ";" | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
@@ -19,7 +22,7 @@
 void Record::function_public() {}
 
 // CHECK: ---
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'Record'
 // CHECK-NEXT: Path:'GlobalNamespace'
 // CHECK-NEXT: Namespace:
@@ -30,12 +33,12 @@
 // CHECK-NEXT:   Filename:'{{.*}}'
 // CHECK-NEXT: TagType: Class
 // CHECK-NEXT: ChildFunctions:
-// CHECK-NEXT:   - USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT:   - USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: Name:'function_public'
 // CHECK-NEXT: Namespace:
 // CHECK-NEXT:   - Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT:   - Type:Namespace
 // CHECK-NEXT: Name:'GlobalNamespace'
 // CHECK-NEXT: DefLocation:
@@ -48,7 +51,7 @@
 // CHECK-NEXT: Parent:
 // CHECK-NEXT: Type:Record
 // CHECK-NEXT: Name:'Record'
-// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}'
+// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: ReturnType:
 // CHECK-NEXT:   Type:
 // CHECK-NEXT: Name:'void'
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,6 +43,7 @@
 #include "llvm

[PATCH] D138961: [clang-doc] Fix warnings about lock_guard.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a project: clang-tools-extra.
Herald added a subscriber: arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Fixes a warning about a potentially unsupported template argument deduction by 
explicitly specifying the template type in std::lock_guard.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138961

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -264,13 +264,13 @@
 
   // Add a reference to this Info in the Index
   {
-std::lock_guard Guard(IndexMutex);
+std::lock_guard Guard(IndexMutex);
 clang::doc::Generator::addInfoToIndex(CDCtx.Idx, Reduced.get().get());
   }
 
   // Save in the result map (needs a lock due to threaded access).
   {
-std::lock_guard Guard(USRToInfoMutex);
+std::lock_guard Guard(USRToInfoMutex);
 USRToInfo[Group.getKey()] = std::move(Reduced.get());
   }
 });


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -264,13 +264,13 @@
 
   // Add a reference to this Info in the Index
   {
-std::lock_guard Guard(IndexMutex);
+std::lock_guard Guard(IndexMutex);
 clang::doc::Generator::addInfoToIndex(CDCtx.Idx, Reduced.get().get());
   }
 
   // Save in the result map (needs a lock due to threaded access).
   {
-std::lock_guard Guard(USRToInfoMutex);
+std::lock_guard Guard(USRToInfoMutex);
 USRToInfo[Group.getKey()] = std::move(Reduced.get());
   }
 });
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a project: clang-tools-extra.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Decodes the template parameters for both functions and records.

For template specializations, records a reference to the main record that the 
specialization is of, and also the parameters that the code supplied to that 
template.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp

Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "Generators.h"
+#include "Representation.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -23,6 +24,7 @@
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumValueInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(TemplateParamInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(TypedefInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(BaseRecordInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr)
@@ -142,6 +144,7 @@
   IO.mapOptional("ChildFunctions", I.Children.Functions);
   IO.mapOptional("ChildEnums", I.Children.Enums);
   IO.mapOptional("ChildTypedefs", I.Children.Typedefs);
+  IO.mapOptional("Template", I.Template);
 }
 
 static void CommentInfoMapping(IO &IO, CommentInfo &I) {
@@ -267,6 +270,28 @@
 // the AS that shouldn't be part of the output. Even though AS_public is the
 // default in the struct, it should be displayed in the YAML output.
 IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
+IO.mapOptional("Template", I.Template);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, TemplateParamInfo &I) {
+IO.mapOptional("Contents", I.Contents);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, TemplateSpecializationInfo &I) {
+IO.mapOptional("SpecializationOf", I.SpecializationOf);
+IO.mapOptional("Params", I.Params);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, TemplateInfo &I) {
+IO.mapOptional("Params", I.Params);
+IO.mapOptional("Specialization", I.Specialization,
+   Optional());
   }
 };
 
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -249,7 +249,8 @@
   } else {
 IT = InfoType::IT_default;
   }
-  return TypeInfo(Reference(getUSRForDecl(TD), TD->getNameAsString(), IT,
+  return TypeInfo(Reference(getUSRForDecl(TD),
+/*TD->getNameAsString()*/ T.getAsString(), IT,
 getInfoRelativePath(TD)));
 }
 
@@ -405,10 +406,7 @@
   for (const ParmVarDecl *P : D->parameters()) {
 FieldTypeInfo &FieldInfo = I.Params.emplace_back(
 getTypeInfoForType(P->getOriginalType()), P->getNameAsString());
-
-if (const Expr *DefaultArg = P->getDefaultArg()) {
-  FieldInfo.DefaultValue = getSourceCode(D, DefaultArg->getSourceRange());
-}
+FieldInfo.DefaultValue = getSourceCode(D, P->getDefaultArgRange());
   }
 }
 
@@ -476,6 +474,30 @@
 InfoType::IT_namespace);
 }
 
+void PopulateTemplateParameters(llvm::Optional &TemplateInfo,
+const clang::Decl *D) {
+  if (const TemplateParameterList *ParamList =
+  D->getDescribedTemplateParams()) {
+if (!TemplateInfo) {
+  TemplateInfo.emplace();
+}
+for (const NamedDecl *ND : *ParamList) {
+  TemplateInfo->Params.emplace_back(
+  getSourceCode(ND, ND->getSourceRange()));
+}
+  }
+}
+
+TemplateParamInfo TemplateArgumentToInfo(const clang::Decl *D,
+ const TemplateArgument &Arg) {
+  // The TemplateArgument's pretty printing handles all the normal cases
+  // well enough for our requirements.
+  std::string Str;
+  llvm::raw_string_ostream Stream(Str);
+  Arg.print(PrintingPolicy(D->getLangOpts()), Stream, false);
+  return TemplateParamInfo(Str);
+}
+
 template 
 static void populateInfo(Info &I, const T *D, const FullComment *C,
  bool &IsInAnonymousNamespace) {
@@ -508,6 +530,26 @@
  IsInAnonymousNamespace);
   I.ReturnType = getTypeInfoForType(D->getReturnType());
   parseParameters(I, D);
+
+  Popul

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 479772.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp

Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "Generators.h"
+#include "Representation.h"
 #include "llvm/Support/YAMLTraits.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -23,6 +24,7 @@
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(EnumValueInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(TemplateParamInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(TypedefInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(BaseRecordInfo)
 LLVM_YAML_IS_SEQUENCE_VECTOR(std::unique_ptr)
@@ -142,6 +144,7 @@
   IO.mapOptional("ChildFunctions", I.Children.Functions);
   IO.mapOptional("ChildEnums", I.Children.Enums);
   IO.mapOptional("ChildTypedefs", I.Children.Typedefs);
+  IO.mapOptional("Template", I.Template);
 }
 
 static void CommentInfoMapping(IO &IO, CommentInfo &I) {
@@ -174,6 +177,7 @@
   static void mapping(IO &IO, Reference &Ref) {
 IO.mapOptional("Type", Ref.RefType, InfoType::IT_default);
 IO.mapOptional("Name", Ref.Name, SmallString<16>());
+IO.mapOptional("QualName", Ref.QualName, SmallString<16>());
 IO.mapOptional("USR", Ref.USR, SymbolID());
 IO.mapOptional("Path", Ref.Path, SmallString<128>());
   }
@@ -267,6 +271,28 @@
 // the AS that shouldn't be part of the output. Even though AS_public is the
 // default in the struct, it should be displayed in the YAML output.
 IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
+IO.mapOptional("Template", I.Template);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, TemplateParamInfo &I) {
+IO.mapOptional("Contents", I.Contents);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, TemplateSpecializationInfo &I) {
+IO.mapOptional("SpecializationOf", I.SpecializationOf);
+IO.mapOptional("Params", I.Params);
+  }
+};
+
+template <> struct MappingTraits {
+  static void mapping(IO &IO, TemplateInfo &I) {
+IO.mapOptional("Params", I.Params);
+IO.mapOptional("Specialization", I.Specialization,
+   Optional());
   }
 };
 
Index: clang-tools-extra/clang-doc/Serialize.cpp
===
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -239,7 +239,7 @@
 TypeInfo getTypeInfoForType(const QualType &T) {
   const TagDecl *TD = getTagDeclForType(T);
   if (!TD)
-return TypeInfo(Reference(SymbolID(), T.getAsString()));
+return TypeInfo(Reference(SymbolID(), T.getAsString(), T.getAsString()));
 
   InfoType IT;
   if (dyn_cast(TD)) {
@@ -249,7 +249,7 @@
   } else {
 IT = InfoType::IT_default;
   }
-  return TypeInfo(Reference(getUSRForDecl(TD), TD->getNameAsString(), IT,
+  return TypeInfo(Reference(getUSRForDecl(TD), TD->getNameAsString(), T.getAsString(), IT,
 getInfoRelativePath(TD)));
 }
 
@@ -280,12 +280,12 @@
 //
 // See MakeAndInsertIntoParent().
 static void InsertChild(ScopeChildren &Scope, const NamespaceInfo &Info) {
-  Scope.Namespaces.emplace_back(Info.USR, Info.Name, InfoType::IT_namespace,
+  Scope.Namespaces.emplace_back(Info.USR, Info.Name, Info.Name, InfoType::IT_namespace,
 getInfoRelativePath(Info.Namespace));
 }
 
 static void InsertChild(ScopeChildren &Scope, const RecordInfo &Info) {
-  Scope.Records.emplace_back(Info.USR, Info.Name, InfoType::IT_record,
+  Scope.Records.emplace_back(Info.USR, Info.Name, Info.Name, InfoType::IT_record,
  getInfoRelativePath(Info.Namespace));
 }
 
@@ -405,10 +405,7 @@
   for (const ParmVarDecl *P : D->parameters()) {
 FieldTypeInfo &FieldInfo = I.Params.emplace_back(
 getTypeInfoForType(P->getOriginalType()), P->getNameAsString());
-
-if (const Expr *DefaultArg = P->getDefaultArg()) {
-  FieldInfo.DefaultValue = getSourceCode(D, DefaultArg->getSourceRange());
-}
+FieldInfo.DefaultValue = getSourceCode(D, P->getDefaultArgRange());
   }
 }
 
@@ -424,9 +421,10 @@
 if (const auto *Ty = B.getType()->getAs()) {
   const TemplateDecl *D = Ty->getTemplateName().getAsTemplateDecl();
   I.Parents.emplace_back(getUSRForDecl(D), B.getType().getAsString(),
+  

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 480251.
brettw added a reviewer: paulkirth.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,10 +28,11 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace,
- "path/to/A/Namespace");
+  I.Children.Namespaces.emplace_back(
+  EmptySID, "ChildNamespace", InfoType::IT_namespace,
+  "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace");
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path::to::A::Namespace::ChildStruct",
   "path/to/A/Namespace");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
@@ -53,13 +54,16 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+QualName:'path::to::A::Namespace::ChildNamespace'
 Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::Namespace::ChildStruct'
 Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
@@ -83,8 +87,7 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
- AccessSpecifier::AS_private);
+  I.Members.emplace_back(TypeInfo("int"), "X", AccessSpecifier::AS_private);
 
   // Member documentation.
   CommentInfo TopComment;
@@ -103,15 +106,15 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().Children.Functions.emplace_back();
   I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
-"path/to/G");
+"path::to::G::G", "path/to/G");
 
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
+  "path::to::A::r::ChildStruct", "path/to/A/r");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
   I.Children.Enums.emplace_back();
@@ -131,6 +134,7 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 DefLocation:
   LineNumber:  10
   Filename:'test.cpp'
@@ -142,7 +146,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'X'
 Access:  Private
 Description:
@@ -161,7 +165,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'N'
 Access:  Private
 ChildFunctions:
@@ -178,10 +182,12 @@
 VirtualParents:
   - Type:Record
 Name:'G'
+QualName:'path::to::G::G'
 Path:'path/to/G'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::r::ChildStruct'
 Path:'path/to/A/r'
 ChildFunctions:
   - USR: ''
@@ -206,10 +212,9 @@
 
   I.Access = AccessSpecifier::AS_none;
 
-  I.ReturnType = TypeInfo(
-  Reference(EmptySID, "void", InfoType::IT_default, "path/to/void"));
-  I

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 480536.
brettw marked an inline comment as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp
  clang-tools-extra/test/clang-doc/templates.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,10 +28,11 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace,
- "path/to/A/Namespace");
+  I.Children.Namespaces.emplace_back(
+  EmptySID, "ChildNamespace", InfoType::IT_namespace,
+  "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace");
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path::to::A::Namespace::ChildStruct",
   "path/to/A/Namespace");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
@@ -53,13 +54,16 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+QualName:'path::to::A::Namespace::ChildNamespace'
 Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::Namespace::ChildStruct'
 Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
@@ -83,8 +87,7 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
- AccessSpecifier::AS_private);
+  I.Members.emplace_back(TypeInfo("int"), "X", AccessSpecifier::AS_private);
 
   // Member documentation.
   CommentInfo TopComment;
@@ -103,15 +106,15 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().Children.Functions.emplace_back();
   I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
-"path/to/G");
+"path::to::G::G", "path/to/G");
 
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
+  "path::to::A::r::ChildStruct", "path/to/A/r");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
   I.Children.Enums.emplace_back();
@@ -131,6 +134,7 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 DefLocation:
   LineNumber:  10
   Filename:'test.cpp'
@@ -142,7 +146,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'X'
 Access:  Private
 Description:
@@ -161,7 +165,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'N'
 Access:  Private
 ChildFunctions:
@@ -178,10 +182,12 @@
 VirtualParents:
   - Type:Record
 Name:'G'
+QualName:'path::to::G::G'
 Path:'path/to/G'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::r::ChildStruct'
 Path:'path/to/A/r'
 ChildFunctions:
   - USR: ''
@@ -206,1

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw marked an inline comment as done.
brettw added a comment.

New patch up.




Comment at: clang-tools-extra/test/clang-doc/single-file.cpp:19
 // CHECK-NEXT:DefLocation:
-// CHECK-NEXT:  LineNumber:  [[@LINE-8]]
+// CHECK-NEXT:  LineNumber:  11
 // CHECK-NEXT:  Filename:'{{.*}}

I changed the existing test from relative line numbers which don't make sense 
given the structure of this (where the code is mostly static at the top, and 
the YAML is at the bottom which occasionally changes).



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:631
 
+TEST(SerializeTests, emitFunctionTemplate) {
+  EmittedInfoList Infos;

paulkirth wrote:
> I may have missed these cases in the test file, so if they exist just ignore 
> me.
> 
> -  a template w/ no concrete types e.g., `template void 
> GetFoo(T);` This seems like a very common case, so it would be good to have a 
> test explicitly for it. 
> - a nested template, both where the inner template type is known, and where 
> it is not.
> 
I added the nested template example to the unit test. I also added a template 
parameter pack to the e2e test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 480560.
brettw marked 5 inline comments as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp
  clang-tools-extra/test/clang-doc/templates.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,10 +28,11 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace,
- "path/to/A/Namespace");
+  I.Children.Namespaces.emplace_back(
+  EmptySID, "ChildNamespace", InfoType::IT_namespace,
+  "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace");
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path::to::A::Namespace::ChildStruct",
   "path/to/A/Namespace");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
@@ -53,13 +54,16 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+QualName:'path::to::A::Namespace::ChildNamespace'
 Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::Namespace::ChildStruct'
 Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
@@ -83,8 +87,7 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
- AccessSpecifier::AS_private);
+  I.Members.emplace_back(TypeInfo("int"), "X", AccessSpecifier::AS_private);
 
   // Member documentation.
   CommentInfo TopComment;
@@ -103,15 +106,15 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().Children.Functions.emplace_back();
   I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
-"path/to/G");
+"path::to::G::G", "path/to/G");
 
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
+  "path::to::A::r::ChildStruct", "path/to/A/r");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
   I.Children.Enums.emplace_back();
@@ -131,6 +134,7 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 DefLocation:
   LineNumber:  10
   Filename:'test.cpp'
@@ -142,7 +146,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'X'
 Access:  Private
 Description:
@@ -161,7 +165,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'N'
 Access:  Private
 ChildFunctions:
@@ -178,10 +182,12 @@
 VirtualParents:
   - Type:Record
 Name:'G'
+QualName:'path::to::G::G'
 Path:'path/to/G'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::r::ChildStruct'
 Path:'path/to/A/r'
 ChildFunctions:
   - USR: ''
@@ -206,1

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.h:119-123
+  // This variant (that takes no qualified name parameter) uses the Name as the
+  // QualName (very useful in unit tests to reduce verbosity). This can't use
+  // the empty string as a default because the global namespace uses
+  // "GlobalNamespace" as the name, but should have an empty QualName
+  // (corresponding to the name in code).

paulkirth wrote:
> Does this imply that its illegal to use `""` as the name with this 
> constructor? if so, it probably requires at least an assert.
No, I meant something else, clarified.



Comment at: clang-tools-extra/clang-doc/Representation.h:210-212
+  // The literal contents of the code for everything that specifies this
+  // template parameter. This will include all components, so for a template
+  // declaration it could be "typename T = int".

paulkirth wrote:
> I think this is equivalent. Do you mean this contains //all// instantiations 
> of the parameter across the codebase? or for a single declaration?
Clarified


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f6dbb5f1646: [clang-doc] Add template support. (authored by 
brettw).

Changed prior to commit:
  https://reviews.llvm.org/D139154?vs=480560&id=480960#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp
  clang-tools-extra/test/clang-doc/templates.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,10 +28,11 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace,
- "path/to/A/Namespace");
+  I.Children.Namespaces.emplace_back(
+  EmptySID, "ChildNamespace", InfoType::IT_namespace,
+  "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace");
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path::to::A::Namespace::ChildStruct",
   "path/to/A/Namespace");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
@@ -53,13 +54,16 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+QualName:'path::to::A::Namespace::ChildNamespace'
 Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::Namespace::ChildStruct'
 Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
@@ -83,8 +87,7 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
- AccessSpecifier::AS_private);
+  I.Members.emplace_back(TypeInfo("int"), "X", AccessSpecifier::AS_private);
 
   // Member documentation.
   CommentInfo TopComment;
@@ -103,15 +106,15 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().Children.Functions.emplace_back();
   I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
-"path/to/G");
+"path::to::G::G", "path/to/G");
 
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
+  "path::to::A::r::ChildStruct", "path/to/A/r");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
   I.Children.Enums.emplace_back();
@@ -131,6 +134,7 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 DefLocation:
   LineNumber:  10
   Filename:'test.cpp'
@@ -142,7 +146,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'X'
 Access:  Private
 Description:
@@ -161,7 +165,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'N'
 Access:  Private
 ChildFunctions:
@@ -178,10 +182,12 @@
 VirtualParents:
   - Type:Record
 Name:'G'
+QualName:'path::to::G::G'
 Path:'path/to/G'
 ChildRecords:
   - Type:Record
 Name:   

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

brettw updated this revision to Diff 480979.
brettw added a comment.

New patch with llvm::Optional converted to std::optional (previously collided 
with a refactor).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp
  clang-tools-extra/test/clang-doc/templates.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,10 +28,11 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace,
- "path/to/A/Namespace");
+  I.Children.Namespaces.emplace_back(
+  EmptySID, "ChildNamespace", InfoType::IT_namespace,
+  "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace");
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path::to::A::Namespace::ChildStruct",
   "path/to/A/Namespace");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
@@ -53,13 +54,16 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+QualName:'path::to::A::Namespace::ChildNamespace'
 Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::Namespace::ChildStruct'
 Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
@@ -83,8 +87,7 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
- AccessSpecifier::AS_private);
+  I.Members.emplace_back(TypeInfo("int"), "X", AccessSpecifier::AS_private);
 
   // Member documentation.
   CommentInfo TopComment;
@@ -103,15 +106,15 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().Children.Functions.emplace_back();
   I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
-"path/to/G");
+"path::to::G::G", "path/to/G");
 
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
+  "path::to::A::r::ChildStruct", "path/to/A/r");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
   I.Children.Enums.emplace_back();
@@ -131,6 +134,7 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 DefLocation:
   LineNumber:  10
   Filename:'test.cpp'
@@ -142,7 +146,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'X'
 Access:  Private
 Description:
@@ -161,7 +165,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'N'
 Access:  Private
 ChildFunctions:
@@ -178,10 +182,12 @@
 VirtualParents:
   - Type:Record
 Name:'G'
+QualName:'path::to::G::G'
 Path:'path/to/G'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::r::ChildStruct'
 Path:'path/to/A/r'
 ChildFun

[PATCH] D139154: [clang-doc] Add template support

[Brett Wilson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4a68babd9973: [clang-doc] Add template support. (authored by 
brettw).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139154/new/

https://reviews.llvm.org/D139154

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/test/clang-doc/single-file.cpp
  clang-tools-extra/test/clang-doc/templates.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -28,10 +28,11 @@
   I.Path = "path/to/A";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace,
- "path/to/A/Namespace");
+  I.Children.Namespaces.emplace_back(
+  EmptySID, "ChildNamespace", InfoType::IT_namespace,
+  "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace");
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+  "path::to::A::Namespace::ChildStruct",
   "path/to/A/Namespace");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
@@ -53,13 +54,16 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 ChildNamespaces:
   - Type:Namespace
 Name:'ChildNamespace'
+QualName:'path::to::A::Namespace::ChildNamespace'
 Path:'path/to/A/Namespace'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::Namespace::ChildStruct'
 Path:'path/to/A/Namespace'
 ChildFunctions:
   - USR: ''
@@ -83,8 +87,7 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
-  I.Members.emplace_back(TypeInfo("int", "path/to/int"), "X",
- AccessSpecifier::AS_private);
+  I.Members.emplace_back(TypeInfo("int"), "X", AccessSpecifier::AS_private);
 
   // Member documentation.
   CommentInfo TopComment;
@@ -103,15 +106,15 @@
AccessSpecifier::AS_public, true);
   I.Bases.back().Children.Functions.emplace_back();
   I.Bases.back().Children.Functions.back().Name = "InheritedFunctionOne";
-  I.Bases.back().Members.emplace_back(TypeInfo("int", "path/to/int"), "N",
+  I.Bases.back().Members.emplace_back(TypeInfo("int"), "N",
   AccessSpecifier::AS_private);
   // F is in the global namespace
   I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
-"path/to/G");
+"path::to::G::G", "path/to/G");
 
   I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
-  "path/to/A/r");
+  "path::to::A::r::ChildStruct", "path/to/A/r");
   I.Children.Functions.emplace_back();
   I.Children.Functions.back().Name = "OneFunction";
   I.Children.Enums.emplace_back();
@@ -131,6 +134,7 @@
 Namespace:
   - Type:Namespace
 Name:'A'
+QualName:'A'
 DefLocation:
   LineNumber:  10
   Filename:'test.cpp'
@@ -142,7 +146,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'X'
 Access:  Private
 Description:
@@ -161,7 +165,7 @@
 Members:
   - Type:
   Name:'int'
-  Path:'path/to/int'
+  QualName:'int'
 Name:'N'
 Access:  Private
 ChildFunctions:
@@ -178,10 +182,12 @@
 VirtualParents:
   - Type:Record
 Name:'G'
+QualName:'path::to::G::G'
 Path:'path/to/G'
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
+QualName:'path::to::A::r::ChildStruct'
 Path: