subject:"\[PATCH\] D63663\: \[clang\-doc\] Add html links to the parents of a RecordInfo"

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

2019-06-24 Thread Julie Hockett via Phabricator via cfe-commits

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:21
 
 namespace {
 

Eugene.Zelenko wrote:
> Anonymous namespace is used for functions when LLVM Coding Guidelines tells 
> using static for same purpose.
Actually, since these aren't used outside of this file, please use the `static` 
keyword instead of wrapping them in a namespace.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:58
+if (!R.Path.empty()) {
+  Stream << genLink(R.Name, R.Path + "/" + R.Name + ".html");
+} else {

Use llvm::sys::path to allow for other platforms that use a different separator.



Comment at: clang-tools-extra/clang-doc/MDGenerator.cpp:21
 
-namespace {
+namespace md_generator {
 

Same here, just use static.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:31
+// Example: Given the below, the  path for class C will be <
+// root>/A/B/C
+//

/A/B/C.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:230
   if (const auto *N = dyn_cast(T)) {
 I.Members.emplace_back(getUSRForDecl(T), N->getNameAsString(),
InfoType::IT_enum, F->getNameAsString(),

Also for members



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:255
   if (const auto *N = dyn_cast(T)) {
 I.Params.emplace_back(getUSRForDecl(N), N->getNameAsString(),
   InfoType::IT_enum, P->getNameAsString());

Also for param references, if applicable



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:308-310
+if (const auto *P = getDeclForType(B.getType()))
+  I.VirtualParents.emplace_back(getUSRForDecl(P), P->getNameAsString(),
+InfoType::IT_record);

Can we do the path construction for virtual bases as well, so they can be 
linked too?



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:341-350
   if (const auto *T = getDeclForType(D->getReturnType())) {
 if (dyn_cast(T))
   I.ReturnType =
   TypeInfo(getUSRForDecl(T), T->getNameAsString(), InfoType::IT_enum);
 else if (dyn_cast(T))
   I.ReturnType =
   TypeInfo(getUSRForDecl(T), T->getNameAsString(), 
InfoType::IT_record);

Add path information to these references as well



Comment at: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp:145-146
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};)raw",
+   3, /*Public=*/false, Infos);
 

formatting change?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63663/new/

https://reviews.llvm.org/D63663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

2019-06-24 Thread Diego Astiazarán via Phabricator via cfe-commits

DiegoAstiazaran updated this revision to Diff 206317.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.
Herald added a subscriber: ormris.

Path name generation was moved to serialization phase as @juliehockett 
suggested.
Empty string declaration fixed.
Removed anonymous namespaces.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63663/new/

https://reviews.llvm.org/D63663

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/Generators.cpp
  clang-tools-extra/clang-doc/Generators.h
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/MDGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.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/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

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
@@ -37,7 +37,7 @@
 
   bool VisitNamespaceDecl(const NamespaceDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -48,7 +48,7 @@
 if (dyn_cast(D))
   return true;
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -56,7 +56,7 @@
 
   bool VisitCXXMethodDecl(const CXXMethodDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -64,7 +64,7 @@
 
   bool VisitRecordDecl(const RecordDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -72,7 +72,7 @@
 
   bool VisitEnumDecl(const EnumDecl *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", Public, "");
 if (I)
   EmittedInfos.emplace_back(std::move(I));
 return true;
@@ -142,7 +142,8 @@
   E() {}
 protected:
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};)raw",
+   3, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, "E");
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
@@ -72,7 +72,8 @@
 
   I.Members.emplace_back("int", "X", AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record);
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
+ llvm::SmallString<128>("/path/to"));
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record);
 
   I.ChildRecords.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record);
@@ -89,7 +90,7 @@
   assert(!Err);
   std::string Expected = R"raw(class r
 Defined at line 10 of test.cpp
-Inherits from F, G
+Inherits from F, G
 
 Members
 private int X
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -110,30 +110,8 @@
   return false;
 }
 
-// A function to extract the appropriate path name for a given info's
-// documentation. The path returned is a composite of the parent namespaces as
-// directories plus the decl name as the filename.
-//
-// Example: Given the below, the  path for class C will be <
-// root>/A/B/C.
-//
-// namespace A {
-// namesapce B {
-//
-// class C {};
-//
-// }
-// }
 llvm::Expected>
-getInfoOutputFile(StringRef Root,
-

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

2019-06-21 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:21
 
 namespace {
 

Anonymous namespace is used for functions when LLVM Coding Guidelines tells 
using static for same purpose.



Comment at: clang-tools-extra/clang-doc/HTMLGenerator.cpp:31
+   const llvm::StringMap ) {
+  std::string OptionsText = "";
+  for (auto  : Options) {

You don't need to assign empty string. See [[ 
https://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-string-init.html
 | readability-redundant-string-init ]].


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63663/new/

https://reviews.llvm.org/D63663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

2019-06-21 Thread Julie Hockett via Phabricator via cfe-commits

juliehockett added a comment.

General design thought: could we move the path name construction to 
`Serialize.cpp`? Essentially, move the logic that builds the path name from the 
namespaces and name to the serialize step (you'll have to add the 
`OutDirectory` to the `ClangDocContext` so that you can access it in the 
mapper), and then the `GetInfoOuputFile()` function in `ClangDocMain.cpp` would 
take the `Info.Path` string as a parameter and do the directory construction 
and whatnot.

With that, you could then also add the `Path` field to `Reference`s, so that 
you wouldn't need to pass a map around in the generation phase. What do you 
think?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63663/new/

https://reviews.llvm.org/D63663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

2019-06-21 Thread Diego Astiazarán via Phabricator via cfe-commits

DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: juliehockett, jakehehrlich, lebedev.ri.
DiegoAstiazaran added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.

 tags are added for the parents of records.
The link redirects to the parent's info file.

Depends on D63180 .


https://reviews.llvm.org/D63663

Files:
  clang-tools-extra/clang-doc/Generators.cpp
  clang-tools-extra/clang-doc/Generators.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/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/MDGeneratorTest.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
@@ -39,7 +39,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -89,7 +90,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -148,7 +150,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -194,7 +197,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected =
   R"raw(---
@@ -331,7 +335,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected =
   R"raw(---
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
@@ -38,7 +38,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected = R"raw(# namespace Namespace
 
@@ -101,7 +102,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected = R"raw(# class r
 
@@ -162,7 +164,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected = R"raw(### f
 
@@ -190,7 +193,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected = R"raw(| enum class e |
 
@@ -320,7 +324,8 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected =
   R"raw(### f
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
@@ -9,6 +9,7 @@
 #include "ClangDocTest.h"
 #include "Generators.h"
 #include "Representation.h"
+#include "Serialize.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -38,7 +39,9 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  auto Err = G->generateDocForInfo(, Actual);
+
+  std::map> Infos;
+  auto Err = G->generateDocForInfo(, Actual, Infos);
   assert(!Err);
   std::string Expected = R"raw(namespace Namespace
 
@@ -72,7

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

[PATCH] D63663: [clang-doc] Add html links to the parents of a RecordInfo

5 matches

Site Navigation

Mail list logo

Footer information