[PATCH] D66151: [clang-doc] Fix bitcode writer

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369063: [clang-doc] Fix bitcode writer for access specifiers 
(authored by DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66151?vs=215193=215491#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66151

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

Index: clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -43,6 +43,7 @@
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
   "Namespace");
   I.ChildFunctions.emplace_back();
+  I.ChildFunctions.back().Access = AccessSpecifier::AS_none;
   I.ChildFunctions.back().Name = "OneFunction";
   I.ChildEnums.emplace_back();
   I.ChildEnums.back().Name = "OneEnum";
@@ -228,7 +229,7 @@
   Functions
   
 OneFunction
-OneFunction()
+public OneFunction()
   
   Enums
   
@@ -248,6 +249,8 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
+  I.Access = AccessSpecifier::AS_none;
+
   SmallString<16> PathTo;
   llvm::sys::path::native("path/to", PathTo);
   I.ReturnType = TypeInfo(EmptySID, "float", InfoType::IT_default, PathTo);
@@ -331,6 +334,7 @@
   I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   I.Params.emplace_back("int", "I");
   I.Params.emplace_back("int", "J");
+  I.Access = AccessSpecifier::AS_none;
 
   CommentInfo Top;
   Top.Kind = "FullComment";
Index: clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -34,6 +34,7 @@
   "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";
 
@@ -138,6 +139,7 @@
   - USR: ''
 Name:'OneFunction'
 ReturnType:  {}
+Access:  Public
 ChildEnums:
   - USR: ''
 Name:'OneEnum'
@@ -154,6 +156,8 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
+  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");
@@ -242,6 +246,7 @@
   I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   I.Params.emplace_back("int", "I");
   I.Params.emplace_back("int", "J");
+  I.Access = AccessSpecifier::AS_none;
 
   CommentInfo Top;
   Top.Kind = "FullComment";
Index: clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
@@ -106,6 +106,8 @@
   I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   I.Params.emplace_back("int", "P");
 
+  I.Access = AccessSpecifier::AS_none;
+
   std::string WriteResult = writeInfo();
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector> ReadResults = readInfo(WriteResult, 1);
@@ -126,8 +128,7 @@
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
-  // TODO: fix access
-  // I.Access = AccessSpecifier::AS_private;
+  I.Access = AccessSpecifier::AS_public;
 
   std::string WriteResult = writeInfo();
   EXPECT_TRUE(WriteResult.size() > 0);
Index: clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp
+++ 

[PATCH] D66151: [clang-doc] Fix bitcode writer

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 215193.
DiegoAstiazaran marked an inline comment as done.
DiegoAstiazaran added a comment.

Use getAccess() instead of getAccessUnsafe() for CXXMethodDecl.
Add comments in YAML generator to specify that AS_none is used as the default 
here because it's 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.


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

https://reviews.llvm.org/D66151

Files:
  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/test/clang-doc/single-file-public.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.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
@@ -34,6 +34,7 @@
   "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";
 
@@ -138,6 +139,7 @@
   - USR: ''
 Name:'OneFunction'
 ReturnType:  {}
+Access:  Public
 ChildEnums:
   - USR: ''
 Name:'OneEnum'
@@ -154,6 +156,8 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
+  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");
@@ -242,6 +246,7 @@
   I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   I.Params.emplace_back("int", "I");
   I.Params.emplace_back("int", "J");
+  I.Access = AccessSpecifier::AS_none;
 
   CommentInfo Top;
   Top.Kind = "FullComment";
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
@@ -103,6 +103,7 @@
   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);
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -299,6 +300,7 @@
   F.Name = "F";
   F.ReturnType = TypeInfo(EmptySID, "int", InfoType::IT_default);
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -314,6 +316,7 @@
   F.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back("int", "I");
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -379,6 +382,7 @@
   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.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 
@@ -389,6 +393,7 @@
   ExportedF.ReturnType = TypeInfo(EmptySID, "double", InfoType::IT_default);
   ExportedF.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   ExportedF.Params.emplace_back("double", "y");
+  ExportedF.Access = AccessSpecifier::AS_none;
   ExpectedBWithExportedFunction.ChildFunctions.emplace_back(
   std::move(ExportedF));
   CheckNamespaceInfo(, BWithExportedFunction);
Index: clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
@@ -31,6 +31,7 @@
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
   I.ChildFunctions.emplace_back();
   I.ChildFunctions.back().Name = "OneFunction";
+  

[PATCH] D66151: [clang-doc] Fix bitcode writer

[Julie Hockett via Phabricator via cfe-commits]

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

One comment then LGTM




Comment at: clang-tools-extra/clang-doc/Serialize.cpp:507
   Reference{ParentUSR, Parent->getNameAsString(), InfoType::IT_record};
-  Func.Access = D->getAccess();
+  Func.Access = D->getAccessUnsafe();
 

In a C++ method, it should have an access associated with it, and so AS_none 
isn't a valid value.


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

https://reviews.llvm.org/D66151



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

[PATCH] D66151: [clang-doc] Fix bitcode writer

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 214923.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.

Default value of AccessSpecifier attributes is now AS_public.
Multiple tests modified.


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

https://reviews.llvm.org/D66151

Files:
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/test/clang-doc/single-file-public.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.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
@@ -34,6 +34,7 @@
   "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";
 
@@ -138,6 +139,7 @@
   - USR: ''
 Name:'OneFunction'
 ReturnType:  {}
+Access:  Public
 ChildEnums:
   - USR: ''
 Name:'OneEnum'
@@ -154,6 +156,8 @@
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
+  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");
@@ -242,6 +246,7 @@
   I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   I.Params.emplace_back("int", "I");
   I.Params.emplace_back("int", "J");
+  I.Access = AccessSpecifier::AS_none;
 
   CommentInfo Top;
   Top.Kind = "FullComment";
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
@@ -103,6 +103,7 @@
   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);
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -299,6 +300,7 @@
   F.Name = "F";
   F.ReturnType = TypeInfo(EmptySID, "int", InfoType::IT_default);
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -314,6 +316,7 @@
   F.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   F.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   F.Params.emplace_back("int", "I");
+  F.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 }
@@ -379,6 +382,7 @@
   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.Access = AccessSpecifier::AS_none;
   ExpectedBWithFunction.ChildFunctions.emplace_back(std::move(F));
   CheckNamespaceInfo(, BWithFunction);
 
@@ -389,6 +393,7 @@
   ExportedF.ReturnType = TypeInfo(EmptySID, "double", InfoType::IT_default);
   ExportedF.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
   ExportedF.Params.emplace_back("double", "y");
+  ExportedF.Access = AccessSpecifier::AS_none;
   ExpectedBWithExportedFunction.ChildFunctions.emplace_back(
   std::move(ExportedF));
   CheckNamespaceInfo(, BWithExportedFunction);
Index: clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
@@ -31,6 +31,7 @@
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
   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";
 
@@ -127,7 +128,7 @@
 
 ### OneFunction
 
-* OneFunction()*
+*public  OneFunction()*
 
 
 
@@ -153,6 +154,8 @@
   I.DefLoc = Location(10, 

[PATCH] D66151: [clang-doc] Fix bitcode writer

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: jakehehrlich, juliehockett.
DiegoAstiazaran added a project: clang-tools-extra.

Bitcode writer was not emitting the corresponding record for the Access 
attribute of a FunctionInfo.
This is added and corresponding test is included.


https://reviews.llvm.org/D66151

Files:
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  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
@@ -126,8 +126,7 @@
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
-  // TODO: fix access
-  // I.Access = AccessSpecifier::AS_private;
+  I.Access = AccessSpecifier::AS_private;
 
   std::string WriteResult = writeInfo();
   EXPECT_TRUE(WriteResult.size() > 0);
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -510,6 +510,7 @@
 emitBlock(N, FieldId::F_namespace);
   for (const auto  : I.Description)
 emitBlock(CI);
+  emitRecord(I.Access, FUNCTION_ACCESS);
   emitRecord(I.IsMethod, FUNCTION_IS_METHOD);
   if (I.DefLoc)
 emitRecord(I.DefLoc.getValue(), FUNCTION_DEFLOCATION);


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
@@ -126,8 +126,7 @@
   I.IsMethod = true;
   I.Parent = Reference(EmptySID, "Parent", InfoType::IT_record);
 
-  // TODO: fix access
-  // I.Access = AccessSpecifier::AS_private;
+  I.Access = AccessSpecifier::AS_private;
 
   std::string WriteResult = writeInfo();
   EXPECT_TRUE(WriteResult.size() > 0);
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -510,6 +510,7 @@
 emitBlock(N, FieldId::F_namespace);
   for (const auto  : I.Description)
 emitBlock(CI);
+  emitRecord(I.Access, FUNCTION_ACCESS);
   emitRecord(I.IsMethod, FUNCTION_IS_METHOD);
   if (I.DefLoc)
 emitRecord(I.DefLoc.getValue(), FUNCTION_DEFLOCATION);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits