[PATCH] D66151: [clang-doc] Fix bitcode writer
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&id=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(&I); 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(&I); EXPECT_TRUE(WriteResult.size() > 0); Index: clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp === --- clang-tools-extra/trunk/unittests/clang-doc/SerializeTest.cpp +++ clang-tool
[PATCH] D66151: [clang-doc] Fix bitcode writer
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(&ExpectedBWithFunction, 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(&ExpectedBWithFunction, 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(&ExpectedBWithFunction, 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(&ExpectedBWithFunction, 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(&ExpectedBWithExportedFunction, 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::
[PATCH] D66151: [clang-doc] Fix bitcode writer
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
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(&ExpectedBWithFunction, 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(&ExpectedBWithFunction, 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(&ExpectedBWithFunction, 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(&ExpectedBWithFunction, 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(&ExpectedBWithExportedFunction, 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 @@ ###
[PATCH] D66151: [clang-doc] Fix bitcode writer
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(&I); 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 &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); 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(&I); 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 &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); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits