[PATCH] D134055: [clang-doc] Add support for explicitly typed enums
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeaa7b324d5a2: [clang-doc] Add support for explicitly typed
enums (authored by brettw, committed by paulkirth).
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", "-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);
[PATCH] D134055: [clang-doc] Add support for explicitly typed enums
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] D134055: [clang-doc] Add support for explicitly typed enums
paulkirth added a comment. Sorry, it looks like a pre-submit formatting check failed. can you `git clang-format HEAD~` and re-upload? I can land your change after that. 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
paulkirth accepted this revision. paulkirth added a comment. This revision is now accepted and ready to land. LGTM. 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
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] D134055: [clang-doc] Add support for explicitly typed enums
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
=
