[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang
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
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
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
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
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
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
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
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