[clang-doc] add namespaces to JSON generator #143209
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
evelez7
force-pushed
the
users/evelez7/clang-doc-json-generator
branch
from
8becb23 to
54287df
Compare
evelez7
force-pushed
the
users/evelez7/clang-doc-json-namespaces
branch
3 times, most recently
from
979bfbf to
21a472c
Compare
evelez7
force-pushed
the
users/evelez7/clang-doc-json-generator
branch
from
54287df to
b567c6c
Compare
|
@llvm/pr-subscribers-clang-tools-extra Author: Erick Velez (evelez7) ChangesFull diff: https://github.com/llvm/llvm-project/pull/143209.diff 5 Files Affected:
diff --git a/clang-tools-extra/clang-doc/JSONGenerator.cpp b/clang-tools-extra/clang-doc/JSONGenerator.cpp
index 37773cf3f5651..789951dcc6440 100644
--- a/clang-tools-extra/clang-doc/JSONGenerator.cpp
+++ b/clang-tools-extra/clang-doc/JSONGenerator.cpp
@@ -443,6 +443,41 @@ static void serializeInfo(const RecordInfo &I, json::Object &Obj,
serializeCommonChildren(I.Children, Obj, RepositoryUrl);
}
+static void serializeInfo(const NamespaceInfo &I, json::Object &Obj,
+ std::optional<StringRef> RepositoryUrl) {
+ serializeCommonAttributes(I, Obj, RepositoryUrl);
+ SmallString<64> BasePath = I.getRelativeFilePath("");
+ Obj["NamespacePath"] = BasePath;
+
+ if (!I.Children.Namespaces.empty()) {
+ json::Value NamespacesArray = Array();
+ auto &NamespacesArrayRef = *NamespacesArray.getAsArray();
+ NamespacesArrayRef.reserve(I.Children.Namespaces.size());
+ for (auto &Namespace : I.Children.Namespaces) {
+ json::Value NamespaceVal = Object();
+ auto &NamespaceObj = *NamespaceVal.getAsObject();
+ serializeReference(Namespace, NamespaceObj, BasePath);
+ NamespacesArrayRef.push_back(NamespaceVal);
+ }
+ Obj["Namespaces"] = NamespacesArray;
+ }
+
+ if (!I.Children.Functions.empty()) {
+ json::Value FunctionsArray = Array();
+ auto &FunctionsArrayRef = *FunctionsArray.getAsArray();
+ FunctionsArrayRef.reserve(I.Children.Functions.size());
+ for (const auto &Function : I.Children.Functions) {
+ json::Value FunctionVal = Object();
+ auto &FunctionObj = *FunctionVal.getAsObject();
+ serializeInfo(Function, FunctionObj, RepositoryUrl);
+ FunctionsArrayRef.push_back(FunctionVal);
+ }
+ Obj["Functions"] = FunctionsArray;
+ }
+
+ serializeCommonChildren(I.Children, Obj, RepositoryUrl);
+}
+
Error JSONGenerator::generateDocs(
StringRef RootDir, llvm::StringMap<std::unique_ptr<doc::Info>> Infos,
const ClangDocContext &CDCtx) {
@@ -485,6 +520,7 @@ Error JSONGenerator::generateDocForInfo(Info *I, raw_ostream &OS,
switch (I->IT) {
case InfoType::IT_namespace:
+ serializeInfo(*static_cast<NamespaceInfo *>(I), Obj, CDCtx.RepositoryUrl);
break;
case InfoType::IT_record:
serializeInfo(*static_cast<RecordInfo *>(I), Obj, CDCtx.RepositoryUrl);
diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp
index 462001b3f3027..3cda38115ff7f 100644
--- a/clang-tools-extra/clang-doc/Serialize.cpp
+++ b/clang-tools-extra/clang-doc/Serialize.cpp
@@ -742,6 +742,7 @@ static void populateFunctionInfo(FunctionInfo &I, const FunctionDecl *D,
I.ReturnType = getTypeInfoForType(D->getReturnType(), LO);
I.Prototype = getFunctionPrototype(D);
parseParameters(I, D);
+ I.IsStatic = D->isStatic();
populateTemplateParameters(I.Template, D);
diff --git a/clang-tools-extra/test/clang-doc/json/function-specifiers.cpp b/clang-tools-extra/test/clang-doc/json/function-specifiers.cpp
new file mode 100644
index 0000000000000..7005fb7b3e66e
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/json/function-specifiers.cpp
@@ -0,0 +1,25 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --output=%t --format=json --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.json
+
+static void myFunction() {}
+
+void noExceptFunction() noexcept {}
+
+inline void inlineFunction() {}
+
+extern void externFunction() {}
+
+constexpr void constexprFunction() {}
+
+// CHECK: "Functions": [
+// CHECK-NEXT: {
+// CHECK: "IsStatic": true,
+// COM: FIXME: Emit ExceptionSpecificationType
+// CHECK-NOT: "ExceptionSpecifcation" : "noexcept",
+// COM: FIXME: Emit inline
+// CHECK-NOT: "IsInline": true,
+// COM: FIXME: Emit extern
+// CHECK-NOT: "IsExtern": true,
+// COM: FIXME: Emit constexpr
+// CHECK-NOT: "IsConstexpr": true,
diff --git a/clang-tools-extra/test/clang-doc/json/namespace.cpp b/clang-tools-extra/test/clang-doc/json/namespace.cpp
new file mode 100644
index 0000000000000..ca9fd0a6aa7d1
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/json/namespace.cpp
@@ -0,0 +1,108 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --output=%t --format=json --executor=standalone %s
+// RUN: FileCheck %s < %t/GlobalNamespace/index.json
+
+class MyClass {};
+
+void myFunction(int Param);
+
+namespace NestedNamespace {
+} // namespace NestedNamespace
+
+// FIXME: Global variables are not mapped or serialized.
+static int Global;
+
+enum Color {
+ RED,
+ GREEN,
+ BLUE = 5
+};
+
+typedef int MyTypedef;
+
+// CHECK: {
+// CHECK-NEXT: "Enums": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "Location": {
+// CHECK-NEXT: "Filename": "{{.*}}namespace.cpp",
+// CHECK-NEXT: "LineNumber": 15
+// CHECK-NEXT: },
+// CHECK-NEXT: "Members": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "Name": "RED",
+// CHECK-NEXT: "Value": "0"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "Name": "GREEN",
+// CHECK-NEXT: "Value": "1"
+// CHECK-NEXT: },
+// CHECK-NEXT: {
+// CHECK-NEXT: "Name": "BLUE",
+// CHECK-NEXT: "ValueExpr": "5"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "Name": "Color",
+// CHECK-NEXT: "Scoped": false,
+// CHECK-NEXT: "USR": "{{[0-9A-F]*}}"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "Functions": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "IsStatic": false,
+// CHECK-NEXT: "Name": "myFunction",
+// CHECK-NEXT: "Params": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "Name": "Param",
+// CHECK-NEXT: "Type": "int"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "ReturnType": {
+// CHECK-NEXT: "IsBuiltIn": false,
+// CHECK-NEXT: "IsTemplate": false,
+// CHECK-NEXT: "Name": "void",
+// CHECK-NEXT: "QualName": "void",
+// CHECK-NEXT: "USR": "0000000000000000000000000000000000000000"
+// CHECK-NEXT: },
+// CHECK-NEXT: "USR": "{{[0-9A-F]*}}"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "Name": "",
+// CHECK-NEXT: "NamespacePath": "GlobalNamespace",
+// CHECK-NEXT: "Namespaces": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "Name": "NestedNamespace",
+// CHECK-NEXT: "Path": "",
+// CHECK-NEXT: "QualName": "NestedNamespace",
+// CHECK-NEXT: "USR": "{{[0-9A-F]*}}"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "Records": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "Name": "MyClass",
+// CHECK-NEXT: "Path": "GlobalNamespace",
+// CHECK-NEXT: "QualName": "MyClass",
+// CHECK-NEXT: "USR": "{{[0-9A-F]*}}"
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "Typedefs": [
+// CHECK-NEXT: {
+// CHECK-NEXT: "IsUsing": false,
+// CHECK-NEXT: "Location": {
+// CHECK-NEXT: "Filename": "{{.*}}namespace.cpp",
+// CHECK-NEXT: "LineNumber": 21
+// CHECK-NEXT: },
+// CHECK-NEXT: "Name": "MyTypedef",
+// CHECK-NEXT: "TypeDeclaration": "",
+// CHECK-NEXT: "USR": "{{[0-9A-F]*}}",
+// CHECK-NEXT: "Underlying": {
+// CHECK-NEXT: "IsBuiltIn": false,
+// CHECK-NEXT: "IsTemplate": false,
+// CHECK-NEXT: "Name": "int",
+// CHECK-NEXT: "QualName": "int",
+// CHECK-NEXT: "USR": "0000000000000000000000000000000000000000"
+// CHECK-NEXT: }
+// CHECK-NEXT: }
+// CHECK-NEXT: ],
+// CHECK-NEXT: "USR": "0000000000000000000000000000000000000000"
+// CHECK-NOT: "Variables": [
+// CHECK-NEXT: }
diff --git a/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp
index 24516cbc34a5a..bc468698cf150 100644
--- a/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp
+++ b/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp
@@ -171,5 +171,78 @@ TEST(JSONGeneratorTest, emitRecordJSON) {
})raw";
EXPECT_EQ(Expected, Actual.str());
}
+
+TEST(JSONGeneratorTest, emitNamespaceJSON) {
+ NamespaceInfo I;
+ I.Name = "Namespace";
+ 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::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";
+ I.Children.Functions.back().Access = AccessSpecifier::AS_none;
+ I.Children.Enums.emplace_back();
+ I.Children.Enums.back().Name = "OneEnum";
+
+ auto G = getJSONGenerator();
+ 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({
+ "Enums": [
+ {
+ "Name": "OneEnum",
+ "Scoped": false,
+ "USR": "0000000000000000000000000000000000000000"
+ }
+ ],
+ "Functions": [
+ {
+ "IsStatic": false,
+ "Name": "OneFunction",
+ "ReturnType": {
+ "IsBuiltIn": false,
+ "IsTemplate": false,
+ "Name": "",
+ "QualName": "",
+ "USR": "0000000000000000000000000000000000000000"
+ },
+ "USR": "0000000000000000000000000000000000000000"
+ }
+ ],
+ "Name": "Namespace",
+ "Namespace": [
+ "A"
+ ],
+ "NamespacePath": "path/to/A/Namespace",
+ "Namespaces": [
+ {
+ "Name": "ChildNamespace",
+ "Path": "path/to/A/Namespace",
+ "QualName": "path::to::A::Namespace::ChildNamespace",
+ "USR": "0000000000000000000000000000000000000000"
+ }
+ ],
+ "Path": "path/to/A",
+ "Records": [
+ {
+ "Name": "ChildStruct",
+ "Path": "path/to/A/Namespace",
+ "QualName": "path::to::A::Namespace::ChildStruct",
+ "USR": "0000000000000000000000000000000000000000"
+ }
+ ],
+ "USR": "0000000000000000000000000000000000000000"
+})raw";
+ EXPECT_EQ(Expected, Actual.str());
+}
} // namespace doc
} // namespace clang
|
|
|
||
| // CHECK: "Functions": [ | ||
| // CHECK-NEXT: { | ||
| // CHECK: "IsStatic": true, |
There was a problem hiding this comment.
Don't you need something like CHECK-LABEL to make sure you're matching the correct part of the output? If the name is later in the output, maybe we should move it to make testing easier? Otherwise, I'd guess we would still need to check that we're matching the right property w/ the right entity...
Most of this test is just precommit for the FIXMEs, so it isn't a big deal right now, but I can imagine as the test grows, it may become harder to maintain.
There was a problem hiding this comment.
Yeah this was motivated by convenience. The functions are serialized in order, so I know that the first entry to be checked will be the static function. Hence, the "IsStatic": true check. The contents of an object are serialized alphabetically, so I haven't figured how to check the name first, since that comes after all the Is fields.
That would be useful. I can investigate this more.
There was a problem hiding this comment.
Matching the property and the name after should be OK, though not ideal. IIRC the JSON we emit from llvm-readobj isn't sorted by key, so maybe there is a setting in the JSON builder? Or maybe it's because it uses a JSON stream? I can't quite recall, but it may be worth looking at other subprojects to see how they deal with the issue.
| // CHECK-NEXT: "IsTemplate": false, | ||
| // CHECK-NEXT: "Name": "void", | ||
| // CHECK-NEXT: "QualName": "void", | ||
| // CHECK-NEXT: "USR": "0000000000000000000000000000000000000000" |
There was a problem hiding this comment.
Do builtin types like void and int get fixed USRs? Also, shouldn't IsBuiltIn be true here? The default value is false, but we should set it in Serialize.cpp, right?
There was a problem hiding this comment.
Yes those USRs are fixed.
IsBuiltIn is set in Serialize.cpp and I just checked in gdb. IsBuiltIn is true for void but it gets emitted to its default value. I'm not sure how it works but I have a feeling the bitcode reader doesn't properly serialize everything.
There was a problem hiding this comment.
Hmm, we're probably updating the wrong data structure/copy. Booleans should serialize fine. It could also be that we're doing something silly in a copy constructor or using a bad initializer list.
There was a problem hiding this comment.
I'm fairly certain the reason we are seeing lots of default values is that these values aren't defined in BitecodeWriter.cpp. Every Info field has a corresponding enum thing.
llvm-project/clang-tools-extra/clang-doc/BitcodeWriter.cpp
Lines 196 to 201 in d75e284
And then corresponding values in a couple other places + BitcodeReader.cpp
| "Namespace": [ | ||
| "A" | ||
| ], | ||
| "NamespacePath": "path/to/A/Namespace", |
There was a problem hiding this comment.
Are these file paths going to work on Windows?
There was a problem hiding this comment.
C:\_work\llvm-project\llvm-project\clang-tools-extra\test\clang-doc\json\class.cpp:166:16: error: CHECK-NEXT: expected string not found in input
# | // CHECK-NEXT: "Path": "GlobalNamespace/MyClass",
# | ^
# | <stdin>:169:24: note: scanning from here
# | "Name": "NestedClass",
# | ^
# | <stdin>:170:2: note: possible intended match here
# | "Path": "GlobalNamespace\\MyClass",
# | ^
From the bot, it looks like its not. Depending on how you expect that field to be used, I could see you go one of two ways.
- You want to use the path as a real filesystem path, in which case, you'd update the test to check for either separator.
- It's more an abstract/logical distinction, and you intend to use it similar to a URL, in which case you can always make it POSIX style.
There was a problem hiding this comment.
I've been trying to figure this out and I am inclined to remove this field altogether (in fact I have the changes staged right now). I copied the Mustache behavior from here:
llvm-project/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp
Lines 473 to 475 in 4e68962
And I use this behavior in the initial JSON generator patch as well because Mustache does it for all entities:
llvm-project/clang-tools-extra/clang-doc/JSONGenerator.cpp
Lines 201 to 203 in b567c6c
I think this behavior is incorrect for the JSON generator and a mistake on my part. References already have their own Path fields and I think that's what should be emitted, not a relative file path for linking documents. If the JSON ends up being fed to an HTML generator, then I think the HTML generator should construct its own paths for linking, which will overwrite this field or create a new "link" field. @ilovepi
It was also incorrect because instead of checking for the proper path/to/A/r, the test was emitting checking for the filename link (something like ../index.json) which was again an oversight by me.
As for Windows, yes it's particularly nasty for unit tests because I can't use the nice regex stuff ({{[.*]}} in there, so I'm figuring that out. I don't know how YAML solved it.
There was a problem hiding this comment.
Also this "NamespacePath" is not generated by any other generator besides Mustache and it's not currently used in any Mustache templates.
There was a problem hiding this comment.
OK, that makes sense. Your proposed updates sound good, so lets go with that.
evelez7
force-pushed
the
users/evelez7/clang-doc-json-namespaces
branch
from
21a472c to
dd56536
Compare
evelez7
force-pushed
the
users/evelez7/clang-doc-json-generator
branch
from
b567c6c to
84173b4
Compare
evelez7
force-pushed
the
users/evelez7/clang-doc-json-namespaces
branch
from
dd56536 to
58196a0
Compare
Emit namespaces to JSON. Also adds tests for namespaces and non-member constructs.
Emit namespaces to JSON. Also adds tests for namespaces and non-member constructs.
Emit namespaces to JSON. Also adds tests for namespaces and non-member constructs.