DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: juliehockett, jakehehrlich.
DiegoAstiazaran added a project: clang-tools-extra.
Path is now stored in the references to the child while serializing, then this
path is used to generate the relative path in the HTML generator.
Now some references have paths and some don't so in the reducing phase,
references are now properly merged checking for empty attributes.
Tests added for HTML and YAML generators, merging and serializing.
computeRelativePath function had a bug when the filepath is part of the given
directory; it returned a path that starts with a separator. This has been fixed.
https://reviews.llvm.org/D65987
Files:
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/Representation.cpp
clang-tools-extra/clang-doc/Representation.h
clang-tools-extra/clang-doc/Serialize.cpp
clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
clang-tools-extra/unittests/clang-doc/MergeTest.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
@@ -29,8 +29,9 @@
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace);
- I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+ InfoType::IT_namespace, "path/to/A/Namespace");
+ I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+ "path/to/A/Namespace");
I.ChildFunctions.emplace_back();
I.ChildFunctions.back().Name = "OneFunction";
I.ChildEnums.emplace_back();
@@ -53,9 +54,11 @@
ChildNamespaces:
- Type: Namespace
Name: 'ChildNamespace'
+ Path: 'path/to/A/Namespace'
ChildRecords:
- Type: Record
Name: 'ChildStruct'
+ Path: 'path/to/A/Namespace'
ChildFunctions:
- USR: '0000000000000000000000000000000000000000'
Name: 'OneFunction'
@@ -71,7 +74,7 @@
TEST(YAMLGeneratorTest, emitRecordYAML) {
RecordInfo I;
I.Name = "r";
- I.Path = "path/to/r";
+ I.Path = "path/to/A";
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -85,7 +88,8 @@
I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
"path/to/G");
- I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+ I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+ "path/to/A/r");
I.ChildFunctions.emplace_back();
I.ChildFunctions.back().Name = "OneFunction";
I.ChildEnums.emplace_back();
@@ -101,7 +105,7 @@
R"raw(---
USR: '0000000000000000000000000000000000000000'
Name: 'r'
-Path: 'path/to/r'
+Path: 'path/to/A'
Namespace:
- Type: Namespace
Name: 'A'
@@ -129,6 +133,7 @@
ChildRecords:
- Type: Record
Name: 'ChildStruct'
+ Path: 'path/to/A/r'
ChildFunctions:
- USR: '0000000000000000000000000000000000000000'
Name: 'OneFunction'
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
@@ -407,12 +407,14 @@
RecordInfo *ParentB = InfoAsRecord(Infos[3].get());
RecordInfo ExpectedParentB(EmptySID);
- ExpectedParentB.ChildRecords.emplace_back(EmptySID, "B", InfoType::IT_record);
+ ExpectedParentB.ChildRecords.emplace_back(EmptySID, "B", InfoType::IT_record,
+ "A");
CheckRecordInfo(&ExpectedParentB, ParentB);
NamespaceInfo *ParentC = InfoAsNamespace(Infos[7].get());
NamespaceInfo ExpectedParentC(EmptySID);
- ExpectedParentC.ChildRecords.emplace_back(EmptySID, "C", InfoType::IT_record);
+ ExpectedParentC.ChildRecords.emplace_back(EmptySID, "C", InfoType::IT_record,
+ "@nonymous_namespace");
CheckNamespaceInfo(&ExpectedParentC, ParentC);
}
@@ -431,7 +433,7 @@
NamespaceInfo *ParentB = InfoAsNamespace(Infos[3].get());
NamespaceInfo ExpectedParentB(EmptySID);
ExpectedParentB.ChildNamespaces.emplace_back(EmptySID, "B",
- InfoType::IT_namespace);
+ InfoType::IT_namespace, "A");
CheckNamespaceInfo(&ExpectedParentB, ParentB);
}
Index: clang-tools-extra/unittests/clang-doc/MergeTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-doc/MergeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MergeTest.cpp
@@ -87,7 +87,7 @@
One.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
One.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
- One.ChildRecords.emplace_back(NonEmptySID, "ChildStruct",
+ One.ChildRecords.emplace_back(NonEmptySID, "SharedChildStruct",
InfoType::IT_record);
One.ChildFunctions.emplace_back();
One.ChildFunctions.back().Name = "OneFunction";
@@ -104,8 +104,8 @@
Two.TagType = TagTypeKind::TTK_Class;
- Two.ChildRecords.emplace_back(EmptySID, "OtherChildStruct",
- InfoType::IT_record);
+ Two.ChildRecords.emplace_back(NonEmptySID, "SharedChildStruct",
+ InfoType::IT_record, "path");
Two.ChildFunctions.emplace_back();
Two.ChildFunctions.back().Name = "TwoFunction";
Two.ChildEnums.emplace_back();
@@ -127,10 +127,8 @@
Expected->Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
Expected->VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
- Expected->ChildRecords.emplace_back(NonEmptySID, "ChildStruct",
- InfoType::IT_record);
- Expected->ChildRecords.emplace_back(EmptySID, "OtherChildStruct",
- InfoType::IT_record);
+ Expected->ChildRecords.emplace_back(NonEmptySID, "SharedChildStruct",
+ InfoType::IT_record, "path");
Expected->ChildFunctions.emplace_back();
Expected->ChildFunctions.back().Name = "OneFunction";
Expected->ChildFunctions.back().USR = NonEmptySID;
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===================================================================
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -39,8 +39,9 @@
I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
I.ChildNamespaces.emplace_back(EmptySID, "ChildNamespace",
- InfoType::IT_namespace);
- I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+ InfoType::IT_namespace, "Namespace");
+ I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+ "Namespace");
I.ChildFunctions.emplace_back();
I.ChildFunctions.back().Name = "OneFunction";
I.ChildEnums.emplace_back();
@@ -100,11 +101,15 @@
<h1>namespace Namespace</h1>
<h2 id="Namespaces">Namespaces</h2>
<ul>
- <li>ChildNamespace</li>
+ <li>
+ <a href="Namespace/ChildNamespace.html">ChildNamespace</a>
+ </li>
</ul>
<h2 id="Records">Records</h2>
<ul>
- <li>ChildStruct</li>
+ <li>
+ <a href="Namespace/ChildStruct.html">ChildStruct</a>
+ </li>
</ul>
<h2 id="Functions">Functions</h2>
<div>
@@ -137,7 +142,8 @@
I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, PathTo);
I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
- I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
+ I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record,
+ "X/Y/Z/r");
I.ChildFunctions.emplace_back();
I.ChildFunctions.back().Name = "OneFunction";
I.ChildEnums.emplace_back();
@@ -210,7 +216,9 @@
</ul>
<h2 id="Records">Records</h2>
<ul>
- <li>ChildStruct</li>
+ <li>
+ <a href="r/ChildStruct.html">ChildStruct</a>
+ </li>
</ul>
<h2 id="Functions">Functions</h2>
<div>
Index: clang-tools-extra/clang-doc/Serialize.cpp
===================================================================
--- clang-tools-extra/clang-doc/Serialize.cpp
+++ clang-tools-extra/clang-doc/Serialize.cpp
@@ -392,8 +392,8 @@
auto ParentI = llvm::make_unique<NamespaceInfo>();
ParentI->USR = I->Namespace.empty() ? SymbolID() : I->Namespace[0].USR;
- ParentI->ChildNamespaces.emplace_back(I->USR, I->Name,
- InfoType::IT_namespace);
+ ParentI->ChildNamespaces.emplace_back(I->USR, I->Name, InfoType::IT_namespace,
+ getInfoRelativePath(I->Namespace));
if (I->Namespace.empty())
ParentI->Path = getInfoRelativePath(ParentI->Namespace);
return {std::unique_ptr<Info>{std::move(I)},
@@ -424,7 +424,8 @@
if (I->Namespace.empty()) {
auto ParentI = llvm::make_unique<NamespaceInfo>();
ParentI->USR = SymbolID();
- ParentI->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record);
+ ParentI->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record,
+ getInfoRelativePath(I->Namespace));
ParentI->Path = getInfoRelativePath(ParentI->Namespace);
return {std::unique_ptr<Info>{std::move(I)},
std::unique_ptr<Info>{std::move(ParentI)}};
@@ -434,14 +435,16 @@
case InfoType::IT_namespace: {
auto ParentI = llvm::make_unique<NamespaceInfo>();
ParentI->USR = I->Namespace[0].USR;
- ParentI->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record);
+ ParentI->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record,
+ getInfoRelativePath(I->Namespace));
return {std::unique_ptr<Info>{std::move(I)},
std::unique_ptr<Info>{std::move(ParentI)}};
}
case InfoType::IT_record: {
auto ParentI = llvm::make_unique<RecordInfo>();
ParentI->USR = I->Namespace[0].USR;
- ParentI->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record);
+ ParentI->ChildRecords.emplace_back(I->USR, I->Name, InfoType::IT_record,
+ getInfoRelativePath(I->Namespace));
return {std::unique_ptr<Info>{std::move(I)},
std::unique_ptr<Info>{std::move(ParentI)}};
}
Index: clang-tools-extra/clang-doc/Representation.h
===================================================================
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -131,6 +131,9 @@
std::tie(Other.USR, Other.Name, Other.RefType);
}
+ bool mergeable(const Reference &Other);
+ void merge(Reference &&I);
+
SymbolID USR = SymbolID(); // Unique identifer for referenced decl
SmallString<16> Name; // Name of type (possibly unresolved).
InfoType RefType = InfoType::IT_default; // Indicates the type of this
Index: clang-tools-extra/clang-doc/Representation.cpp
===================================================================
--- clang-tools-extra/clang-doc/Representation.cpp
+++ clang-tools-extra/clang-doc/Representation.cpp
@@ -54,13 +54,15 @@
return -1;
}
-// For References, we don't need to actually merge them, we just don't want
-// duplicates.
void reduceChildren(std::vector<Reference> &Children,
std::vector<Reference> &&ChildrenToMerge) {
for (auto &ChildToMerge : ChildrenToMerge) {
- if (getChildIndexIfExists(Children, ChildToMerge) == -1)
+ int mergeIdx = getChildIndexIfExists(Children, ChildToMerge);
+ if (mergeIdx == -1) {
Children.push_back(std::move(ChildToMerge));
+ continue;
+ }
+ Children[mergeIdx].merge(std::move(ChildToMerge));
}
}
@@ -112,6 +114,20 @@
}
}
+bool Reference::mergeable(const Reference &Other) {
+ return RefType == Other.RefType && USR == Other.USR;
+}
+
+void Reference::merge(Reference &&Other) {
+ assert(mergeable(Other));
+ if (Name.empty())
+ Name = Other.Name;
+ if (Path.empty())
+ Path = Other.Path;
+ if (!IsInGlobalNamespace)
+ IsInGlobalNamespace = Other.IsInGlobalNamespace;
+}
+
void Info::mergeBase(Info &&Other) {
assert(mergeable(Other));
if (USR == EmptySID)
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===================================================================
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -211,13 +211,43 @@
// directory.
static SmallString<128> computeRelativePath(StringRef FilePath,
StringRef Directory) {
+ // If the directory is empty, the relative path to the file will be its
+ // complete path.
+ if (Directory.empty())
+ return FilePath;
+
+ // The relative path is an empty path if both dirs are the same.
+ if (FilePath == Directory)
+ return {};
+
+ // This will handle the cases where FilePath is a dir inside of Directory,
+ // example:
+ // Filepath = "A/B/C"
+ // Directory = "A/B"
+ // The resulting path can be calculated by looking through the parent
+ // directories of FilePath until one of them matches Directory. At this point
+ // the new relative path is determined by removing Directory from FilePath.
StringRef Path = FilePath;
while (!Path.empty()) {
if (Directory == Path)
- return FilePath.substr(Path.size());
+ return FilePath.substr(Directory.size() + 1);
Path = llvm::sys::path::parent_path(Path);
}
+ // This will handle any other case.
+ // The relative path will always be at least one parent directory (..) and a
+ // part of the FilePath.
+ // The loop will look for the parent directory of Directory until it matches
+ // FilePath or until the are no more parent directories. For each parent
+ // directory before exiting the loop a ".." is required. The part
+ // of FilePath that's not in the last parent directory of Directory stored is
+ // then appended to the multiple ".."'s stored.
+ // FIXME: In a case where FilePath and Directory share some directories but
+ // Directory is not a dir inside of FilePath, example:
+ // Filepath = "A/B/D"
+ // Directory = "A/B/C"
+ // The resulting path is "../../../A/B/D" instead of a "../D". It is correct
+ // but it would be better to have the shorter version.
StringRef Dir = Directory;
SmallString<128> Result;
while (!Dir.empty()) {
@@ -271,8 +301,8 @@
}
static std::unique_ptr<HTMLNode>
-genTypeReference(const Reference &Type, StringRef CurrentDirectory,
- llvm::Optional<StringRef> JumpToSection = None) {
+genReference(const Reference &Type, StringRef CurrentDirectory,
+ llvm::Optional<StringRef> JumpToSection = None) {
if (Type.Path.empty() && !Type.IsInGlobalNamespace) {
if (!JumpToSection)
return llvm::make_unique<TextNode>(Type.Name);
@@ -296,7 +326,7 @@
for (const auto &R : Refs) {
if (&R != Refs.begin())
Out.emplace_back(llvm::make_unique<TextNode>(", "));
- Out.emplace_back(genTypeReference(R, CurrentDirectory));
+ Out.emplace_back(genReference(R, CurrentDirectory));
}
return Out;
}
@@ -368,7 +398,7 @@
Access = Access + " ";
auto LIBody = llvm::make_unique<TagNode>(HTMLTag::TAG_LI);
LIBody->Children.emplace_back(llvm::make_unique<TextNode>(Access));
- LIBody->Children.emplace_back(genTypeReference(M.Type, ParentInfoDir));
+ LIBody->Children.emplace_back(genReference(M.Type, ParentInfoDir));
LIBody->Children.emplace_back(llvm::make_unique<TextNode>(" " + M.Name));
ULBody->Children.emplace_back(std::move(LIBody));
}
@@ -377,7 +407,7 @@
static std::vector<std::unique_ptr<TagNode>>
genReferencesBlock(const std::vector<Reference> &References,
- llvm::StringRef Title) {
+ llvm::StringRef Title, StringRef ParentPath) {
if (References.empty())
return {};
@@ -386,9 +416,11 @@
Out.back()->Attributes.try_emplace("id", Title);
Out.emplace_back(llvm::make_unique<TagNode>(HTMLTag::TAG_UL));
auto &ULBody = Out.back();
- for (const auto &R : References)
- ULBody->Children.emplace_back(
- llvm::make_unique<TagNode>(HTMLTag::TAG_LI, R.Name));
+ for (const auto &R : References) {
+ auto LiNode = llvm::make_unique<TagNode>(HTMLTag::TAG_LI);
+ LiNode->Children.emplace_back(genReference(R, ParentPath));
+ ULBody->Children.emplace_back(std::move(LiNode));
+ }
return Out;
}
@@ -437,9 +469,9 @@
Out.emplace_back(llvm::make_unique<TagNode>(HTMLTag::TAG_SPAN));
auto &SpanBody = Out.back();
if (!Index.JumpToSection)
- SpanBody->Children.emplace_back(genTypeReference(Index, InfoPath));
+ SpanBody->Children.emplace_back(genReference(Index, InfoPath));
else
- SpanBody->Children.emplace_back(genTypeReference(
+ SpanBody->Children.emplace_back(genReference(
Index, InfoPath, StringRef{Index.JumpToSection.getValue()}));
}
if (Index.Children.empty())
@@ -536,7 +568,7 @@
llvm::make_unique<TextNode>(Access + " "));
if (I.ReturnType.Type.Name != "") {
FunctionHeader->Children.emplace_back(
- genTypeReference(I.ReturnType.Type, ParentInfoDir));
+ genReference(I.ReturnType.Type, ParentInfoDir));
FunctionHeader->Children.emplace_back(llvm::make_unique<TextNode>(" "));
}
FunctionHeader->Children.emplace_back(
@@ -545,8 +577,7 @@
for (const auto &P : I.Params) {
if (&P != I.Params.begin())
FunctionHeader->Children.emplace_back(llvm::make_unique<TextNode>(", "));
- FunctionHeader->Children.emplace_back(
- genTypeReference(P.Type, ParentInfoDir));
+ FunctionHeader->Children.emplace_back(genReference(P.Type, ParentInfoDir));
FunctionHeader->Children.emplace_back(
llvm::make_unique<TextNode>(" " + P.Name));
}
@@ -577,10 +608,10 @@
Out.emplace_back(genHTML(I.Description));
std::vector<std::unique_ptr<TagNode>> ChildNamespaces =
- genReferencesBlock(I.ChildNamespaces, "Namespaces");
+ genReferencesBlock(I.ChildNamespaces, "Namespaces", I.Path);
AppendVector(std::move(ChildNamespaces), Out);
std::vector<std::unique_ptr<TagNode>> ChildRecords =
- genReferencesBlock(I.ChildRecords, "Records");
+ genReferencesBlock(I.ChildRecords, "Records", I.Path);
AppendVector(std::move(ChildRecords), Out);
std::vector<std::unique_ptr<TagNode>> ChildFunctions =
@@ -639,7 +670,7 @@
genRecordMembersBlock(I.Members, I.Path);
AppendVector(std::move(Members), Out);
std::vector<std::unique_ptr<TagNode>> ChildRecords =
- genReferencesBlock(I.ChildRecords, "Records");
+ genReferencesBlock(I.ChildRecords, "Records", I.Path);
AppendVector(std::move(ChildRecords), Out);
std::vector<std::unique_ptr<TagNode>> ChildFunctions =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
