[PATCH] D64958: [clang-doc] Fix link generation

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367958: [clang-doc] Fix link generation (authored by 
DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64958?vs=213498=213499#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64958

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/trunk/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.cpp
@@ -172,6 +172,8 @@
   {REFERENCE_NAME, {"Name", }},
   {REFERENCE_TYPE, {"RefType", }},
   {REFERENCE_PATH, {"Path", }},
+  {REFERENCE_IS_IN_GLOBAL_NAMESPACE,
+   {"IsInGlobalNamespace", }},
   {REFERENCE_FIELD, {"Field", }}};
   assert(Inits.size() == RecordIdCount);
   for (const auto  : Inits) {
@@ -215,7 +217,7 @@
 // Reference Block
 {BI_REFERENCE_BLOCK_ID,
  {REFERENCE_USR, REFERENCE_NAME, REFERENCE_TYPE, REFERENCE_PATH,
-  REFERENCE_FIELD}}};
+  REFERENCE_IS_IN_GLOBAL_NAMESPACE, REFERENCE_FIELD}}};
 
 // AbbreviationMap
 
@@ -387,6 +389,7 @@
   emitRecord(R.Name, REFERENCE_NAME);
   emitRecord((unsigned)R.RefType, REFERENCE_TYPE);
   emitRecord(R.Path, REFERENCE_PATH);
+  emitRecord(R.IsInGlobalNamespace, REFERENCE_IS_IN_GLOBAL_NAMESPACE);
   emitRecord((unsigned)Field, REFERENCE_FIELD);
 }
 
Index: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
===
--- clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
+++ clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
@@ -292,6 +292,8 @@
 return decodeRecord(R, I->RefType, Blob);
   case REFERENCE_PATH:
 return decodeRecord(R, I->Path, Blob);
+  case REFERENCE_IS_IN_GLOBAL_NAMESPACE:
+return decodeRecord(R, I->IsInGlobalNamespace, Blob);
   case REFERENCE_FIELD:
 return decodeRecord(R, F, Blob);
   default:
Index: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
@@ -109,6 +109,7 @@
   REFERENCE_NAME,
   REFERENCE_TYPE,
   REFERENCE_PATH,
+  REFERENCE_IS_IN_GLOBAL_NAMESPACE,
   REFERENCE_FIELD,
   RI_LAST,
   RI_FIRST = VERSION
Index: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
@@ -247,7 +247,7 @@
 
 static std::unique_ptr genTypeReference(const Reference ,
   StringRef CurrentDirectory) {
-  if (Type.Path.empty())
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
 return llvm::make_unique(Type.Name);
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
Index: clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
@@ -156,6 +156,7 @@
 IO.mapOptional("Name", Ref.Name, SmallString<16>());
 IO.mapOptional("USR", Ref.USR, SymbolID());
 IO.mapOptional("Path", Ref.Path, SmallString<128>());
+IO.mapOptional("IsInGlobalNamespace", Ref.IsInGlobalNamespace, false);
   }
 };
 
Index: clang-tools-extra/trunk/clang-doc/Representation.h
===
--- clang-tools-extra/trunk/clang-doc/Representation.h
+++ clang-tools-extra/trunk/clang-doc/Representation.h
@@ -114,11 +114,17 @@
 struct Reference {
   Reference() = default;
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  // An empty path means the info is in the global namespace because the path is
+  // a composite of the parent namespaces.
+  Reference(llvm::StringRef Name, StringRef Path)
+  : Name(Name), Path(Path), IsInGlobalNamespace(Path.empty()) {}
   Reference(SymbolID USR, StringRef Name, InfoType IT)
   : USR(USR), Name(Name), RefType(IT) {}
+  // An empty path means the info is in the global namespace because the path is
+  // a composite of the parent namespaces.
   

[PATCH] D64958: [clang-doc] Fix link generation

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213498.
DiegoAstiazaran marked 2 inline comments as done.
DiegoAstiazaran added a comment.

Fix comments.


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

https://reviews.llvm.org/D64958

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/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.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
@@ -80,7 +80,8 @@
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "path/to/F");
+  // F is in the global namespace
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
@@ -120,7 +121,7 @@
 Parents:
   - Type:Record
 Name:'F'
-Path:'path/to/F'
+IsInGlobalNamespace: true
 VirtualParents:
   - Type:Record
 Name:'G'
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -156,6 +156,7 @@
 IO.mapOptional("Name", Ref.Name, SmallString<16>());
 IO.mapOptional("USR", Ref.USR, SymbolID());
 IO.mapOptional("Path", Ref.Path, SmallString<128>());
+IO.mapOptional("IsInGlobalNamespace", Ref.IsInGlobalNamespace, false);
   }
 };
 
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -114,11 +114,17 @@
 struct Reference {
   Reference() = default;
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  // An empty path means the info is in the global namespace because the path is
+  // a composite of the parent namespaces.
+  Reference(llvm::StringRef Name, StringRef Path)
+  : Name(Name), Path(Path), IsInGlobalNamespace(Path.empty()) {}
   Reference(SymbolID USR, StringRef Name, InfoType IT)
   : USR(USR), Name(Name), RefType(IT) {}
+  // An empty path means the info is in the global namespace because the path is
+  // a composite of the parent namespaces.
   Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
-  : USR(USR), Name(Name), RefType(IT), Path(Path) {}
+  : USR(USR), Name(Name), RefType(IT), Path(Path),
+IsInGlobalNamespace(Path.empty()) {}
 
   bool operator==(const Reference ) const {
 return std::tie(USR, Name, RefType) ==
@@ -130,8 +136,12 @@
   InfoType RefType = InfoType::IT_default; // Indicates the type of this
// Reference (namespace, record,
// function, enum, default).
-  llvm::SmallString<128> Path; // Path of directory where the clang-doc
-   // generated file will be saved
+  // Path of directory where the clang-doc generated file will be saved
+  // (possibly unresolved)
+  llvm::SmallString<128> Path;
+  // Indicates if the info's parent is the global namespace, or if the info is
+  // the global namespace
+  bool IsInGlobalNamespace = false;
 };
 
 // A base struct for TypeInfos
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -245,7 +245,7 @@
 
 static std::unique_ptr genTypeReference(const Reference ,
   StringRef CurrentDirectory) {
-  if (Type.Path.empty())
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
 return llvm::make_unique(Type.Name);
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/clang-doc/BitcodeWriter.h
@@ -109,6 +109,7 @@
   REFERENCE_NAME,
   REFERENCE_TYPE,
   REFERENCE_PATH,
+  REFERENCE_IS_IN_GLOBAL_NAMESPACE,
   REFERENCE_FIELD,
   RI_LAST,
   RI_FIRST = VERSION
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp

[PATCH] D64958: [clang-doc] Fix link generation

[Julie Hockett via Phabricator via cfe-commits]

juliehockett accepted this revision.
juliehockett added a comment.

LGTM with a comment correction




Comment at: clang-tools-extra/clang-doc/Representation.h:117
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.

s/globalnamespace/global namespace



Comment at: clang-tools-extra/clang-doc/Representation.h:123
   : USR(USR), Name(Name), RefType(IT) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.

s/globalnamespace/global namespace


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

https://reviews.llvm.org/D64958



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

[PATCH] D64958: [clang-doc] Fix link generation

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213128.
DiegoAstiazaran marked 3 inline comments as done.
DiegoAstiazaran added a comment.

Fix comments


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

https://reviews.llvm.org/D64958

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/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.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
@@ -80,7 +80,8 @@
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "path/to/F");
+  // F is in the global namespace
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "");
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
@@ -120,7 +121,7 @@
 Parents:
   - Type:Record
 Name:'F'
-Path:'path/to/F'
+IsInGlobalNamespace: true
 VirtualParents:
   - Type:Record
 Name:'G'
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -156,6 +156,7 @@
 IO.mapOptional("Name", Ref.Name, SmallString<16>());
 IO.mapOptional("USR", Ref.USR, SymbolID());
 IO.mapOptional("Path", Ref.Path, SmallString<128>());
+IO.mapOptional("IsInGlobalNamespace", Ref.IsInGlobalNamespace, false);
   }
 };
 
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -114,11 +114,17 @@
 struct Reference {
   Reference() = default;
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.
+  Reference(llvm::StringRef Name, StringRef Path)
+  : Name(Name), Path(Path), IsInGlobalNamespace(Path.empty()) {}
   Reference(SymbolID USR, StringRef Name, InfoType IT)
   : USR(USR), Name(Name), RefType(IT) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.
   Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
-  : USR(USR), Name(Name), RefType(IT), Path(Path) {}
+  : USR(USR), Name(Name), RefType(IT), Path(Path),
+IsInGlobalNamespace(Path.empty()) {}
 
   bool operator==(const Reference ) const {
 return std::tie(USR, Name, RefType) ==
@@ -130,8 +136,12 @@
   InfoType RefType = InfoType::IT_default; // Indicates the type of this
// Reference (namespace, record,
// function, enum, default).
-  llvm::SmallString<128> Path; // Path of directory where the clang-doc
-   // generated file will be saved
+  // Path of directory where the clang-doc generated file will be saved
+  // (possibly unresolved)
+  llvm::SmallString<128> Path;
+  // Indicates if the info's parent is the global namespace, or if the info is
+  // the global namespace
+  bool IsInGlobalNamespace = false;
 };
 
 // A base struct for TypeInfos
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -245,7 +245,7 @@
 
 static std::unique_ptr genTypeReference(const Reference ,
   StringRef CurrentDirectory) {
-  if (Type.Path.empty())
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
 return llvm::make_unique(Type.Name);
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ clang-tools-extra/clang-doc/BitcodeWriter.h
@@ -109,6 +109,7 @@
   REFERENCE_NAME,
   REFERENCE_TYPE,
   REFERENCE_PATH,
+  REFERENCE_IS_IN_GLOBAL_NAMESPACE,
   REFERENCE_FIELD,
   RI_LAST,
   RI_FIRST = VERSION
Index: clang-tools-extra/clang-doc/BitcodeWriter.cpp

[PATCH] D64958: [clang-doc] Fix link generation

[Jake Ehrlich via Phabricator via cfe-commits]

jakehehrlich accepted this revision.
jakehehrlich added a comment.
This revision is now accepted and ready to land.

The code looks fine to me but I don't understand the global details of this. 
Hopefully Julie can still review things from there but if not we don't get 
timely reviews then we can still land this.




Comment at: clang-tools-extra/clang-doc/Representation.h:140-141
+  llvm::SmallString<128>
+  Path; // Path of directory where the clang-doc generated file will be
+// saved (possibly unresolved)
+  bool IsInGlobalNamespace =

Here and below perhaps favor putting the comments above the line rather than to 
the side to make the formatter produce nicer looking code.



Comment at: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp:83-84
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "path/to/F");
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
+ ""); // F is in the global namespace
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,

Maybe put the comment above this line so that the formatter doesn't get all 
wonky.


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

https://reviews.llvm.org/D64958



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

[PATCH] D64958: [clang-doc] Fix link generation

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 211979.
DiegoAstiazaran edited the summary of this revision.
DiegoAstiazaran added a comment.

Change attribute used to check special case of global namespace; IsPathValid 
changed to IsInGlobalNamespace. This attribute is true when its first parent is 
the global namespace or if the info is the global namespace,


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

https://reviews.llvm.org/D64958

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/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.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
@@ -80,7 +80,8 @@
   I.Members.emplace_back("int", "path/to/int", "X",
  AccessSpecifier::AS_private);
   I.TagType = TagTypeKind::TTK_Class;
-  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record, "path/to/F");
+  I.Parents.emplace_back(EmptySID, "F", InfoType::IT_record,
+ ""); // F is in the global namespace
   I.VirtualParents.emplace_back(EmptySID, "G", InfoType::IT_record,
 "path/to/G");
 
@@ -120,7 +121,7 @@
 Parents:
   - Type:Record
 Name:'F'
-Path:'path/to/F'
+IsInGlobalNamespace: true
 VirtualParents:
   - Type:Record
 Name:'G'
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -156,6 +156,7 @@
 IO.mapOptional("Name", Ref.Name, SmallString<16>());
 IO.mapOptional("USR", Ref.USR, SymbolID());
 IO.mapOptional("Path", Ref.Path, SmallString<128>());
+IO.mapOptional("IsInGlobalNamespace", Ref.IsInGlobalNamespace, false);
   }
 };
 
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -114,11 +114,17 @@
 struct Reference {
   Reference() = default;
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.
+  Reference(llvm::StringRef Name, StringRef Path)
+  : Name(Name), Path(Path), IsInGlobalNamespace(Path.empty()) {}
   Reference(SymbolID USR, StringRef Name, InfoType IT)
   : USR(USR), Name(Name), RefType(IT) {}
+  // An empty path means the info is in the globalnamespace because the path is
+  // a composite of the parent namespaces.
   Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
-  : USR(USR), Name(Name), RefType(IT), Path(Path) {}
+  : USR(USR), Name(Name), RefType(IT), Path(Path),
+IsInGlobalNamespace(Path.empty()) {}
 
   bool operator==(const Reference ) const {
 return std::tie(USR, Name, RefType) ==
@@ -130,8 +136,12 @@
   InfoType RefType = InfoType::IT_default; // Indicates the type of this
// Reference (namespace, record,
// function, enum, default).
-  llvm::SmallString<128> Path; // Path of directory where the clang-doc
-   // generated file will be saved
+  llvm::SmallString<128>
+  Path; // Path of directory where the clang-doc generated file will be
+// saved (possibly unresolved)
+  bool IsInGlobalNamespace =
+  false; // Indicates if the info's parent is the global namespace, or if
+ // the info is the global namespace
 };
 
 // A base struct for TypeInfos
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -245,7 +245,7 @@
 
 static std::unique_ptr genTypeReference(const Reference ,
   StringRef CurrentDirectory) {
-  if (Type.Path.empty())
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
 return llvm::make_unique(Type.Name);
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
Index: clang-tools-extra/clang-doc/BitcodeWriter.h
===
--- clang-tools-extra/clang-doc/BitcodeWriter.h
+++ 

[PATCH] D64958: [clang-doc] Fix link generation

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran added a comment.

In D64958#1601228 , @juliehockett 
wrote:

> Under what circumstances, exactly, is the path not set where you would need 
> to create a link despite that? Is it only in the global namespace case? If 
> so, could you special-case that and ignore other empty paths?


It's not specifically in the GlobalNamespace info file. It happens in that one 
but also in any info whose parent namespace is the global namespace.


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

https://reviews.llvm.org/D64958



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

[PATCH] D64958: [clang-doc] Fix link generation

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran marked an inline comment as done.
DiegoAstiazaran added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.h:137
+// saved (possibly unresolved)
+  bool IsPathValid = false; // Indicates if a value has been assigned to Path,
+// it could be an empty string

juliehockett wrote:
> Is this field actually set anywhere? 
Yes, in the constructors where a path is assigned.


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

https://reviews.llvm.org/D64958



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

[PATCH] D64958: [clang-doc] Fix link generation

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

Under what circumstances, exactly, is the path not set where you would need to 
create a link despite that? Is it only in the global namespace case? If so, 
could you special-case that and ignore other empty paths?




Comment at: clang-tools-extra/clang-doc/Representation.h:137
+// saved (possibly unresolved)
+  bool IsPathValid = false; // Indicates if a value has been assigned to Path,
+// it could be an empty string

Is this field actually set anywhere? 


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

https://reviews.llvm.org/D64958



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

[PATCH] D64958: [clang-doc] Fix link generation

[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.

Before making a link to a reference it is required to check that the reference 
has a path (eg. primitives won't have paths).
This was done by checking if the path was empty; that worked because when 
generating paths the outdirectory was included, so if a path was assigned it 
had that outdirectory at least.
The path generation was changed, it's now only the namespaces without the 
outdirectory. So if the info is in the global namespace the path would be empty 
and the old check wouldn't work as expected.
A new attribute has been added to the Reference struct that indicates if the 
object has a valid path, this is false by default and changed to true if a path 
is assigned to it.


https://reviews.llvm.org/D64958

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/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.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
@@ -115,16 +115,19 @@
   - Type:
   Name:'int'
   Path:'path/to/int'
+  IsPathValid: true
 Name:'X'
 Access:  Private
 Parents:
   - Type:Record
 Name:'F'
 Path:'path/to/F'
+IsPathValid: true
 VirtualParents:
   - Type:Record
 Name:'G'
 Path:'path/to/G'
+IsPathValid: true
 ChildRecords:
   - Type:Record
 Name:'ChildStruct'
@@ -181,11 +184,13 @@
   - Type:
   Name:'int'
   Path:'path/to/int'
+  IsPathValid: true
 Name:'P'
 ReturnType:
   Type:
 Name:'void'
 Path:'path/to/void'
+IsPathValid: true
 ...
 )raw";
   EXPECT_EQ(Expected, Actual.str());
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -156,6 +156,7 @@
 IO.mapOptional("Name", Ref.Name, SmallString<16>());
 IO.mapOptional("USR", Ref.USR, SymbolID());
 IO.mapOptional("Path", Ref.Path, SmallString<128>());
+IO.mapOptional("IsPathValid", Ref.IsPathValid, false);
   }
 };
 
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -114,11 +114,12 @@
 struct Reference {
   Reference() = default;
   Reference(llvm::StringRef Name) : Name(Name) {}
-  Reference(llvm::StringRef Name, StringRef Path) : Name(Name), Path(Path) {}
+  Reference(llvm::StringRef Name, StringRef Path)
+  : Name(Name), Path(Path), IsPathValid(true) {}
   Reference(SymbolID USR, StringRef Name, InfoType IT)
   : USR(USR), Name(Name), RefType(IT) {}
   Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
-  : USR(USR), Name(Name), RefType(IT), Path(Path) {}
+  : USR(USR), Name(Name), RefType(IT), Path(Path), IsPathValid(true) {}
 
   bool operator==(const Reference ) const {
 return std::tie(USR, Name, RefType) ==
@@ -130,8 +131,11 @@
   InfoType RefType = InfoType::IT_default; // Indicates the type of this
// Reference (namespace, record,
// function, enum, default).
-  llvm::SmallString<128> Path; // Path of directory where the clang-doc
-   // generated file will be saved
+  llvm::SmallString<128>
+  Path; // Path of directory where the clang-doc generated file will be
+// saved (possibly unresolved)
+  bool IsPathValid = false; // Indicates if a value has been assigned to Path,
+// it could be an empty string
 };
 
 // A base struct for TypeInfos
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -249,7 +249,7 @@
 
 static std::unique_ptr genTypeReference(const Reference ,
   StringRef CurrentDirectory) {
-  if (Type.Path.empty())
+  if (!Type.IsPathValid)
 return llvm::make_unique(Type.Name);
   llvm::SmallString<128> Path =