[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-22 Thread Jan Korous via Phabricator via cfe-commits
jkorous added inline comments.



Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323
+  // A documentation URL has an ID and path size.
+  if (Record.size() != 2)
+return SDError::MalformedDiagnosticRecord;

owenv wrote:
> owenv wrote:
> > jkorous wrote:
> > > I am just wondering what happens with the ID. Shouldn't we pass it too?
> > I think the ID should be `Record[0]` in the call t 
> > visitDocumentationURLRecord below. I based this on the handling of category 
> > records.
> Sorry, I don't know how to reply without marking this as done
Got it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126



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


[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-21 Thread Owen Voorhees via Phabricator via cfe-commits
owenv marked an inline comment as done.
owenv added inline comments.



Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323
+  // A documentation URL has an ID and path size.
+  if (Record.size() != 2)
+return SDError::MalformedDiagnosticRecord;

owenv wrote:
> jkorous wrote:
> > I am just wondering what happens with the ID. Shouldn't we pass it too?
> I think the ID should be `Record[0]` in the call t 
> visitDocumentationURLRecord below. I based this on the handling of category 
> records.
Sorry, I don't know how to reply without marking this as done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126



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


[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-21 Thread Owen Voorhees via Phabricator via cfe-commits
owenv marked an inline comment as done.
owenv added a comment.

@jkorous I'd like to add support for urls in clang-emitted diagnostics too, but 
I can't really commit to a timeframe for getting that done atm. I'm hoping to 
have some time to work on it in the next few weeks.




Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323
+  // A documentation URL has an ID and path size.
+  if (Record.size() != 2)
+return SDError::MalformedDiagnosticRecord;

jkorous wrote:
> I am just wondering what happens with the ID. Shouldn't we pass it too?
I think the ID should be `Record[0]` in the call t visitDocumentationURLRecord 
below. I based this on the handling of category records.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126



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


[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-21 Thread Owen Voorhees via Phabricator via cfe-commits
owenv updated this revision to Diff 265592.
owenv added a comment.

- Update a few comments
- Simplify test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Frontend/SerializedDiagnosticReader.h
  clang/include/clang/Frontend/SerializedDiagnostics.h
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Frontend/SerializedDiagnosticReader.cpp
  clang/test/Misc/Inputs/serialized-diags-documentation.dia
  clang/test/Misc/serialized-diags-documentation.c
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndexDiagnostic.cpp
  clang/tools/libclang/CIndexDiagnostic.h
  clang/tools/libclang/CXLoadedDiagnostic.cpp
  clang/tools/libclang/CXLoadedDiagnostic.h
  clang/tools/libclang/CXStoredDiagnostic.cpp
  clang/tools/libclang/libclang.exports

Index: clang/tools/libclang/libclang.exports
===
--- clang/tools/libclang/libclang.exports
+++ clang/tools/libclang/libclang.exports
@@ -219,6 +219,7 @@
 clang_getDiagnosticCategory
 clang_getDiagnosticCategoryName
 clang_getDiagnosticCategoryText
+clang_getDiagnosticDocumentationURLs
 clang_getDiagnosticFixIt
 clang_getDiagnosticInSet
 clang_getDiagnosticLocation
Index: clang/tools/libclang/CXStoredDiagnostic.cpp
===
--- clang/tools/libclang/CXStoredDiagnostic.cpp
+++ clang/tools/libclang/CXStoredDiagnostic.cpp
@@ -109,3 +109,7 @@
   return cxstring::createDup(Hint.CodeToInsert);
 }
 
+CXStringSet *CXStoredDiagnostic::getDocumentationURLs() const {
+  // Documentation URLs are not yet implemented for native Clang diagnostics.
+  return cxstring::createSet({});
+}
Index: clang/tools/libclang/CXLoadedDiagnostic.h
===
--- clang/tools/libclang/CXLoadedDiagnostic.h
+++ clang/tools/libclang/CXLoadedDiagnostic.h
@@ -58,6 +58,9 @@
   CXString getFixIt(unsigned FixIt,
 CXSourceRange *ReplacementRange) const override;
 
+  /// Return the documentation URLs associated with the diagnostic.
+  virtual CXStringSet *getDocumentationURLs() const override;
+
   static bool classof(const CXDiagnosticImpl *D) {
 return D->getKind() == LoadedDiagnosticKind;
   }
@@ -82,6 +85,7 @@
 
   std::vector Ranges;
   std::vector > FixIts;
+  std::vector DocumentationURLs;
   const char *Spelling;
   llvm::StringRef DiagOption;
   llvm::StringRef CategoryText;
Index: clang/tools/libclang/CXLoadedDiagnostic.cpp
===
--- clang/tools/libclang/CXLoadedDiagnostic.cpp
+++ clang/tools/libclang/CXLoadedDiagnostic.cpp
@@ -41,7 +41,8 @@
   Strings Categories;
   Strings WarningFlags;
   Strings FileNames;
-  
+  Strings DocumentationURLs;
+
   FileSystemOptions FO;
   FileManager FakeFiles;
   llvm::DenseMap Files;
@@ -145,6 +146,10 @@
   return cxstring::createRef(FixIts[FixIt].second);
 }
 
+CXStringSet *CXLoadedDiagnostic::getDocumentationURLs() const {
+  return cxstring::createSet(DocumentationURLs);
+}
+
 void CXLoadedDiagnostic::decodeLocation(CXSourceLocation location,
 CXFile *file,
 unsigned int *line,
@@ -233,6 +238,10 @@
   visitSourceRangeRecord(const serialized_diags::Location ,
  const serialized_diags::Location ) override;
 
+  std::error_code visitDocumentationURLRecord(unsigned ID,
+  StringRef Path) override;
+  std::error_code visitDocumentationRecord(unsigned ID) override;
+
 public:
   DiagLoader(enum CXLoadDiag_Error *e, CXString *es)
   : SerializedDiagnosticReader(), error(e), errorString(es) {
@@ -347,6 +356,15 @@
   return std::error_code();
 }
 
+std::error_code DiagLoader::visitDocumentationURLRecord(unsigned ID,
+StringRef Path) {
+  // FIXME: Why do we care about long strings?
+  if (Path.size() > 65536)
+return reportInvalidFile("Out-of-bounds string in documentation URL.");
+  TopDiags->DocumentationURLs[ID] = TopDiags->copyString(Path);
+  return std::error_code();
+}
+
 std::error_code
 DiagLoader::visitSourceRangeRecord(const serialized_diags::Location ,
const serialized_diags::Location ) {
@@ -372,6 +390,12 @@
   return std::error_code();
 }
 
+std::error_code DiagLoader::visitDocumentationRecord(unsigned ID) {
+  CurrentDiags.back()->DocumentationURLs.push_back(
+  TopDiags->DocumentationURLs[ID]);
+  return std::error_code();
+}
+
 std::error_code DiagLoader::visitDiagnosticRecord(
 unsigned Severity, const serialized_diags::Location ,
 unsigned Category, unsigned Flag, StringRef Message) {
Index: clang/tools/libclang/CIndexDiagnostic.h

[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-21 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment.

Hi Owen,
Do you plan to land the functionality for emitting documentation URLs in clang 
too?




Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323
+  // A documentation URL has an ID and path size.
+  if (Record.size() != 2)
+return SDError::MalformedDiagnosticRecord;

I am just wondering what happens with the ID. Shouldn't we pass it too?



Comment at: clang/test/Misc/serialized-diags-doumentation.c:4
+// CHECK: hello.swift:1:1: error: non-nominal type '(Int, Int)' cannot be 
extended [] []
+// CHECK: [Documentation URL: 
/Users/owenvoorhees/Documents/Development/swift-source/build/Ninja-ReleaseAssert/swift-macosx-x86_64/share/doc/swift/diagnostics/nominal-types.md]

Tip - you can use regexes in CHECKs.
https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-regex-matching-syntax

I think we should change this to something like:
```
// CHECK: [Documentation URL: {{.*}}/doc/swift/diagnostics/nominal-types.md]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126



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


[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-20 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment.

+ Argyrios for libclang changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126



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


[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-20 Thread Owen Voorhees via Phabricator via cfe-commits
owenv added a comment.

Reviewers added are a best guess based on arc cover. Sorry if I made a mistake, 
it's my first time contributing to LLVM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126



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


[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

2020-05-20 Thread Owen Voorhees via Phabricator via cfe-commits
owenv updated this revision to Diff 265294.
owenv added a comment.

Updated terminology to generalize the feature so Clang diagnostics can adopt it 
in the future.
Also switched from local paths to URLs so local and remote documentation can 
both be supported if desired


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80126

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Frontend/SerializedDiagnosticReader.h
  clang/include/clang/Frontend/SerializedDiagnostics.h
  clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
  clang/lib/Frontend/SerializedDiagnosticReader.cpp
  clang/test/Misc/Inputs/serialized-diags-documentation.dia
  clang/test/Misc/serialized-diags-doumentation.c
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndexDiagnostic.cpp
  clang/tools/libclang/CIndexDiagnostic.h
  clang/tools/libclang/CXLoadedDiagnostic.cpp
  clang/tools/libclang/CXLoadedDiagnostic.h
  clang/tools/libclang/CXStoredDiagnostic.cpp
  clang/tools/libclang/libclang.exports

Index: clang/tools/libclang/libclang.exports
===
--- clang/tools/libclang/libclang.exports
+++ clang/tools/libclang/libclang.exports
@@ -219,6 +219,7 @@
 clang_getDiagnosticCategory
 clang_getDiagnosticCategoryName
 clang_getDiagnosticCategoryText
+clang_getDiagnosticDocumentationURLs
 clang_getDiagnosticFixIt
 clang_getDiagnosticInSet
 clang_getDiagnosticLocation
Index: clang/tools/libclang/CXStoredDiagnostic.cpp
===
--- clang/tools/libclang/CXStoredDiagnostic.cpp
+++ clang/tools/libclang/CXStoredDiagnostic.cpp
@@ -109,3 +109,7 @@
   return cxstring::createDup(Hint.CodeToInsert);
 }
 
+CXStringSet *CXStoredDiagnostic::getDocumentationURLs() const {
+  // Documentation URLs are not yet implemented for native Clang diagnostics.
+  return cxstring::createSet({});
+}
Index: clang/tools/libclang/CXLoadedDiagnostic.h
===
--- clang/tools/libclang/CXLoadedDiagnostic.h
+++ clang/tools/libclang/CXLoadedDiagnostic.h
@@ -58,6 +58,9 @@
   CXString getFixIt(unsigned FixIt,
 CXSourceRange *ReplacementRange) const override;
 
+  /// Return the documentation URLs associated with the diagnostic.
+  virtual CXStringSet *getDocumentationURLs() const override;
+
   static bool classof(const CXDiagnosticImpl *D) {
 return D->getKind() == LoadedDiagnosticKind;
   }
@@ -82,6 +85,7 @@
 
   std::vector Ranges;
   std::vector > FixIts;
+  std::vector DocumentationURLs;
   const char *Spelling;
   llvm::StringRef DiagOption;
   llvm::StringRef CategoryText;
Index: clang/tools/libclang/CXLoadedDiagnostic.cpp
===
--- clang/tools/libclang/CXLoadedDiagnostic.cpp
+++ clang/tools/libclang/CXLoadedDiagnostic.cpp
@@ -41,7 +41,8 @@
   Strings Categories;
   Strings WarningFlags;
   Strings FileNames;
-  
+  Strings DocumentationURLs;
+
   FileSystemOptions FO;
   FileManager FakeFiles;
   llvm::DenseMap Files;
@@ -145,6 +146,10 @@
   return cxstring::createRef(FixIts[FixIt].second);
 }
 
+CXStringSet *CXLoadedDiagnostic::getDocumentationURLs() const {
+  return cxstring::createSet(DocumentationURLs);
+}
+
 void CXLoadedDiagnostic::decodeLocation(CXSourceLocation location,
 CXFile *file,
 unsigned int *line,
@@ -233,6 +238,10 @@
   visitSourceRangeRecord(const serialized_diags::Location ,
  const serialized_diags::Location ) override;
 
+  std::error_code visitDocumentationURLRecord(unsigned ID,
+  StringRef Path) override;
+  std::error_code visitDocumentationRecord(unsigned ID) override;
+
 public:
   DiagLoader(enum CXLoadDiag_Error *e, CXString *es)
   : SerializedDiagnosticReader(), error(e), errorString(es) {
@@ -347,6 +356,15 @@
   return std::error_code();
 }
 
+std::error_code DiagLoader::visitDocumentationURLRecord(unsigned ID,
+StringRef Path) {
+  // FIXME: Why do we care about long strings?
+  if (Path.size() > 65536)
+return reportInvalidFile("Out-of-bounds string in documentation URL.");
+  TopDiags->DocumentationURLs[ID] = TopDiags->copyString(Path);
+  return std::error_code();
+}
+
 std::error_code
 DiagLoader::visitSourceRangeRecord(const serialized_diags::Location ,
const serialized_diags::Location ) {
@@ -372,6 +390,12 @@
   return std::error_code();
 }
 
+std::error_code DiagLoader::visitDocumentationRecord(unsigned ID) {
+  CurrentDiags.back()->DocumentationURLs.push_back(
+  TopDiags->DocumentationURLs[ID]);
+  return std::error_code();
+}
+
 std::error_code DiagLoader::visitDiagnosticRecord(
 unsigned Severity, const