[clang-doc] Fix bitcode writer for access specifiers

DiegoAstiazaran · DiegoAstiazaran · commit 6a29ae4bde96 · 2019-08-15T23:04:27.000Z
Bitcode writer was not emitting the corresponding record for the Access attribute of a FunctionInfo. This has been added. AS_none was being used as the default value for any AcesssSpecifier attribute (in FunctionInfo and MemberTypeInfo), this has been changed to AS_public because this is the enum value that evaluates to 0. The bitcode writer doesn't write values that are 0 so if an attribute was set to AS_public, this value is not written and after reading the bitcode it would have the default value which is AS_none. This is why the default value is now AS_public. Differential Revision: https://reviews.llvm.org/D66151 llvm-svn: 369063
diff --git a/clang-tools-extra/clang-doc/BitcodeWriter.cpp b/clang-tools-extra/clang-doc/BitcodeWriter.cpp
@@ -510,6 +510,7 @@ void ClangDocBitcodeWriter::emitBlock(const FunctionInfo &I) {
     emitBlock(N, FieldId::F_namespace);
   for (const auto &CI : I.Description)
     emitBlock(CI);
+  emitRecord(I.Access, FUNCTION_ACCESS);
   emitRecord(I.IsMethod, FUNCTION_IS_METHOD);
   if (I.DefLoc)
     emitRecord(I.DefLoc.getValue(), FUNCTION_DEFLOCATION);
diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h
@@ -198,10 +198,11 @@ struct MemberTypeInfo : public FieldTypeInfo {
            std::tie(Other.Type, Other.Name, Other.Access);
   }
 
-  AccessSpecifier Access = AccessSpecifier::AS_none; // Access level associated
-                                                     // with this info (public,
-                                                     // protected, private,
-                                                     // none).
+  // Access level associated with this info (public, protected, private, none).
+  // AS_public is set as default because the bitcode writer requires the enum
+  // 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;
 };
 
 struct Location {
@@ -312,7 +313,10 @@ struct FunctionInfo : public SymbolInfo {
   TypeInfo ReturnType;   // Info about the return type of this function.
   llvm::SmallVector<FieldTypeInfo, 4> Params; // List of parameters.
   // Access level for this method (public, private, protected, none).
-  AccessSpecifier Access = AccessSpecifier::AS_none;
+  // AS_public is set as default because the bitcode writer requires the enum
+  // 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;
 };
 
 // TODO: Expand to allow for documenting templating, inheritance access,
diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp
@@ -463,12 +463,11 @@ emitInfo(const FunctionDecl *D, const FullComment *FC, int LineNumber,
   bool IsInAnonymousNamespace = false;
   populateFunctionInfo(Func, D, FC, LineNumber, File, IsFileInRootDir,
                        IsInAnonymousNamespace);
+  Func.Access = clang::AccessSpecifier::AS_none;
   if (PublicOnly && ((IsInAnonymousNamespace ||
                       !isPublic(D->getAccess(), D->getLinkageInternal()))))
     return {};
 
-  Func.Access = clang::AccessSpecifier::AS_none;
-
   // Wrap in enclosing scope
   auto ParentI = std::make_unique<NamespaceInfo>();
   if (!Func.Namespace.empty())
diff --git a/clang-tools-extra/clang-doc/YAMLGenerator.cpp b/clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -174,6 +174,9 @@ template <> struct MappingTraits<FieldTypeInfo> {
 template <> struct MappingTraits<MemberTypeInfo> {
   static void mapping(IO &IO, MemberTypeInfo &I) {
     FieldTypeInfoMapping(IO, I);
+    // clang::AccessSpecifier::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.
     IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
   }
 };
@@ -218,6 +221,9 @@ template <> struct MappingTraits<FunctionInfo> {
     IO.mapOptional("Parent", I.Parent, Reference());
     IO.mapOptional("Params", I.Params);
     IO.mapOptional("ReturnType", I.ReturnType);
+    // clang::AccessSpecifier::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.
     IO.mapOptional("Access", I.Access, clang::AccessSpecifier::AS_none);
   }
 };
diff --git a/clang-tools-extra/test/clang-doc/single-file-public.cpp b/clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -46,4 +46,5 @@ void Record::function_public() {}
 // CHECK-NEXT:     ReturnType:
 // CHECK-NEXT:       Type:
 // CHECK-NEXT:         Name:            'void'
+// CHECK-NEXT:     Access:			Public
 // CHECK-NEXT: ...
diff --git a/clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp b/clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
@@ -106,6 +106,8 @@ TEST(BitcodeTest, emitFunctionInfoBitcode) {
   I.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
   I.Params.emplace_back("int", "P");
 
+  I.Access = AccessSpecifier::AS_none;
+
   std::string WriteResult = writeInfo(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
   std::vector<std::unique_ptr<Info>> ReadResults = readInfo(WriteResult, 1);
@@ -126,8 +128,7 @@ TEST(BitcodeTest, emitMethodInfoBitcode) {
   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(&I);
   EXPECT_TRUE(WriteResult.size() > 0);
diff --git a/clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -43,6 +43,7 @@ TEST(HTMLGeneratorTest, emitNamespaceHTML) {
   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 @@ TEST(HTMLGeneratorTest, emitRecordHTML) {
   <h2 id="Functions">Functions</h2>
   <div>
     <h3 id="0000000000000000000000000000000000000000">OneFunction</h3>
-    <p>OneFunction()</p>
+    <p>public OneFunction()</p>
   </div>
   <h2 id="Enums">Enums</h2>
   <div>
@@ -248,6 +249,8 @@ TEST(HTMLGeneratorTest, emitFunctionHTML) {
   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 @@ TEST(HTMLGeneratorTest, emitCommentHTML) {
   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";
diff --git a/clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp
@@ -31,6 +31,7 @@ TEST(MDGeneratorTest, emitNamespaceMD) {
   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 @@ ChildStruct
 
 ### OneFunction
 
-* OneFunction()*
+*public  OneFunction()*
 
 
 
@@ -153,6 +154,8 @@ TEST(MDGeneratorTest, emitFunctionMD) {
   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);
   I.Params.emplace_back("int", "P");
   I.IsMethod = true;
@@ -213,6 +216,7 @@ TEST(MDGeneratorTest, emitCommentMD) {
   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";
diff --git a/clang-tools-extra/unittests/clang-doc/SerializeTest.cpp b/clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -103,6 +103,7 @@ TEST(SerializeTest, emitNamespaceInfo) {
   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(&ExpectedBWithFunction, BWithFunction);
 }
@@ -299,6 +300,7 @@ TEST(SerializeTest, emitPublicFunctionInternalInfo) {
   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(&ExpectedBWithFunction, BWithFunction);
 }
@@ -314,6 +316,7 @@ TEST(SerializeTest, emitInlinedFunctionInfo) {
   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(&ExpectedBWithFunction, BWithFunction);
 }
@@ -379,6 +382,7 @@ export double exportedModuleFunction(double y);)raw",
   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(&ExpectedBWithFunction, BWithFunction);
 
@@ -389,6 +393,7 @@ export double exportedModuleFunction(double y);)raw",
   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(&ExpectedBWithExportedFunction, BWithExportedFunction);
diff --git a/clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -34,6 +34,7 @@ TEST(YAMLGeneratorTest, emitNamespaceYAML) {
                               "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 @@ TagType:         Class
   - USR:             '0000000000000000000000000000000000000000'
     Name:            'OneFunction'
     ReturnType:      {}
+    Access:          Public
 ChildEnums:
   - USR:             '0000000000000000000000000000000000000000'
     Name:            'OneEnum'
@@ -154,6 +156,8 @@ TEST(YAMLGeneratorTest, emitFunctionYAML) {
   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 @@ TEST(YAMLGeneratorTest, emitCommentYAML) {
   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";
