[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
This revision was automatically updated to reflect the committed changes. steakhal marked an inline comment as done. Closed by commit rG744e2a3e2232: [analyzer] ClangSA should tablegen doc urls refering to the main doc page (authored by steakhal). Changed prior to commit: https://reviews.llvm.org/D121387?vs=417344=423577#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 Files: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif clang/utils/TableGen/ClangSACheckersEmitter.cpp Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp === --- clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -75,23 +75,18 @@ } static std::string getCheckerDocs(const Record ) { - StringRef LandingPage; const BitsInit *BI = R.getValueAsBitsInit("Documentation"); if (!BI) PrintFatalError(R.getLoc(), "missing Documentation<...> member for " + getCheckerFullName()); - uint64_t V = getValueFromBitsInit(BI, R); - if (V == 1) -LandingPage = "available_checks.html"; - else if (V == 2) -LandingPage = "alpha_checks.html"; - - if (LandingPage.empty()) + // Ignore 'Documentation' checkers. + if (getValueFromBitsInit(BI, R) == 0) return ""; - return (llvm::Twine("https://clang-analyzer.llvm.org/;) + LandingPage + "#" + - getCheckerFullName()) + std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower(); + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); } Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif === --- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif +++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif @@ -451,7 +451,7 @@ "fullDescription": { "text": "Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)" }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#core.CallAndMessage;, + "helpUri": "https://clang.llvm.org/docs/analyzer/checkers.html#core-callandmessage;, "id": "core.CallAndMessage", "name": "core.CallAndMessage" }, @@ -459,7 +459,7 @@ "fullDescription": { "text": "Check for division by zero" }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#core.DivideZero;, + "helpUri": "https://clang.llvm.org/docs/analyzer/checkers.html#core-dividezero;, "id": "core.DivideZero", "name": "core.DivideZero" }, @@ -467,7 +467,7 @@ "fullDescription": { "text": "Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free()." }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#unix.Malloc;, + "helpUri": "https://clang.llvm.org/docs/analyzer/checkers.html#unix-malloc;, "id": "unix.Malloc", "name": "unix.Malloc" } Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp === --- clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -75,23 +75,18 @@ } static std::string getCheckerDocs(const Record ) { - StringRef LandingPage; const BitsInit *BI = R.getValueAsBitsInit("Documentation"); if (!BI) PrintFatalError(R.getLoc(), "missing Documentation<...> member for " + getCheckerFullName()); - uint64_t V = getValueFromBitsInit(BI, R); - if (V == 1) -LandingPage = "available_checks.html"; - else if (V == 2) -LandingPage = "alpha_checks.html"; - - if (LandingPage.empty()) + // Ignore 'Documentation' checkers. + if (getValueFromBitsInit(BI, R) == 0) return ""; - return (llvm::Twine("https://clang-analyzer.llvm.org/;) + LandingPage + "#" + - getCheckerFullName()) + std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower(); + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); } Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif === ---
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
ASDenysPetrov accepted this revision. ASDenysPetrov added a comment. Thanks! LGTM. Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:87-90 + std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower(); + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); IMO it's a bit messy here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
whisperity added a comment. (Side note: you should avoid the list-expansion syntax in URLs because browsers do not understand them and result in links that are not leading anywhere.) I like this. We should continue getting to the point where the documentation is having a uniform layout. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
steakhal marked an inline comment as done. steakhal added a comment. Now it should be much smaller. Sorry for not doing this in the first place. Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:90-91 + + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); martong wrote: > > This patch will ensure that the doc urls produced by tablegen for the > > ClangSA, will use the new url. Nothing else will be changed. > > If that was the case then we'd need only this hunk of change plus the test > file. Perhaps it would make sense to either split this to more patches > - Adding `Sep` > - Handling missing documentation > - new url > Or please change the description of the patch about these changes. Thanks, done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
steakhal updated this revision to Diff 417344. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 Files: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif clang/utils/TableGen/ClangSACheckersEmitter.cpp Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp === --- clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -75,23 +75,18 @@ } static std::string getCheckerDocs(const Record ) { - StringRef LandingPage; const BitsInit *BI = R.getValueAsBitsInit("Documentation"); if (!BI) PrintFatalError(R.getLoc(), "missing Documentation for " + getCheckerFullName()); - uint64_t V = getValueFromBitsInit(BI, R); - if (V == 1) -LandingPage = "available_checks.html"; - else if (V == 2) -LandingPage = "alpha_checks.html"; - - if (LandingPage.empty()) + // Ignore 'Documentation' checkers. + if (getValueFromBitsInit(BI, R) == 0) return ""; - return (llvm::Twine("https://clang-analyzer.llvm.org/;) + LandingPage + "#" + - getCheckerFullName()) + std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower(); + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); } Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif === --- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif +++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif @@ -451,7 +451,7 @@ "fullDescription": { "text": "Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)" }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#core.CallAndMessage;, + "helpUri": "https://clang.llvm.org/docs/analyzer/checkers.html#core-callandmessage;, "id": "core.CallAndMessage", "name": "core.CallAndMessage" }, @@ -459,7 +459,7 @@ "fullDescription": { "text": "Check for division by zero" }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#core.DivideZero;, + "helpUri": "https://clang.llvm.org/docs/analyzer/checkers.html#core-dividezero;, "id": "core.DivideZero", "name": "core.DivideZero" }, @@ -467,7 +467,7 @@ "fullDescription": { "text": "Check for memory leaks, double free, and use-after-free problems. Traces memory managed by malloc()/free()." }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#unix.Malloc;, + "helpUri": "https://clang.llvm.org/docs/analyzer/checkers.html#unix-malloc;, "id": "unix.Malloc", "name": "unix.Malloc" } Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp === --- clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -75,23 +75,18 @@ } static std::string getCheckerDocs(const Record ) { - StringRef LandingPage; const BitsInit *BI = R.getValueAsBitsInit("Documentation"); if (!BI) PrintFatalError(R.getLoc(), "missing Documentation for " + getCheckerFullName()); - uint64_t V = getValueFromBitsInit(BI, R); - if (V == 1) -LandingPage = "available_checks.html"; - else if (V == 2) -LandingPage = "alpha_checks.html"; - - if (LandingPage.empty()) + // Ignore 'Documentation' checkers. + if (getValueFromBitsInit(BI, R) == 0) return ""; - return (llvm::Twine("https://clang-analyzer.llvm.org/;) + LandingPage + "#" + - getCheckerFullName()) + std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower(); + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); } Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif === --- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif +++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif @@ -451,7 +451,7 @@ "fullDescription": { "text": "Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)" }, - "helpUri":
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
martong added inline comments. Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:90-91 + + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); > This patch will ensure that the doc urls produced by tablegen for the > ClangSA, will use the new url. Nothing else will be changed. If that was the case then we'd need only this hunk of change plus the test file. Perhaps it would make sense to either split this to more patches - Adding `Sep` - Handling missing documentation - new url Or please change the description of the patch about these changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
steakhal added a comment. I'd like to reach a wider consensus regarding this patch. @aaron.ballman @NoQ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
steakhal updated this revision to Diff 415747. steakhal added a comment. Fix the single case when the `Sep` was not forwarded. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 Files: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif clang/utils/TableGen/ClangSACheckersEmitter.cpp Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp === --- clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -24,37 +24,39 @@ // Static Analyzer Checkers Tables generation //===--===// -static std::string getPackageFullName(const Record *R); +static std::string getPackageFullName(const Record *R, StringRef Sep = "."); -static std::string getParentPackageFullName(const Record *R) { - std::string name; +static std::string getParentPackageFullName(const Record *R, +StringRef Sep = ".") { if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage"))) -name = getPackageFullName(DI->getDef()); - return name; +return getPackageFullName(DI->getDef(), Sep); + return ""; } -static std::string getPackageFullName(const Record *R) { - std::string name = getParentPackageFullName(R); - if (!name.empty()) -name += "."; - assert(!R->getValueAsString("PackageName").empty()); - name += R->getValueAsString("PackageName"); - return name; +static std::string getPackageFullName(const Record *R, StringRef Sep) { + std::string ParentPkgName = getParentPackageFullName(R, Sep); + StringRef PackageName = R->getValueAsString("PackageName"); + assert(!PackageName.empty()); + + if (ParentPkgName.empty()) +return PackageName.str(); + return (Twine{ParentPkgName} + Sep + PackageName).str(); } -static std::string getCheckerFullName(const Record *R) { - std::string name = getParentPackageFullName(R); - if (!name.empty()) -name += "."; - assert(!R->getValueAsString("CheckerName").empty()); - name += R->getValueAsString("CheckerName"); - return name; +static std::string getCheckerFullName(const Record *R, StringRef Sep = ".") { + std::string ParentPkgName = getParentPackageFullName(R, Sep); + StringRef CheckerName = R->getValueAsString("CheckerName"); + assert(!CheckerName.empty()); + + if (ParentPkgName.empty()) +return CheckerName.str(); + return (Twine{ParentPkgName} + Sep + CheckerName).str(); } static std::string getStringValue(const Record , StringRef field) { if (StringInit *SI = dyn_cast(R.getValueInit(field))) return std::string(SI->getValue()); - return std::string(); + return ""; } // Calculates the integer value representing the BitsInit object @@ -74,20 +76,19 @@ } static std::string getCheckerDocs(const Record ) { - StringRef LandingPage; - if (BitsInit *BI = R.getValueAsBitsInit("Documentation")) { -uint64_t V = getValueFromBitsInit(BI, R); -if (V == 1) - LandingPage = "available_checks.html"; -else if (V == 2) - LandingPage = "alpha_checks.html"; - } - - if (LandingPage.empty()) + const BitsInit *BI = R.getValueAsBitsInit("Documentation"); + if (!BI) +PrintFatalError(R.getLoc(), +"missing Documentation for " + getCheckerFullName()); + + // Ignore 'Documentation' checkers. + if (getValueFromBitsInit(BI, R) == 0) return ""; - return (llvm::Twine("https://clang-analyzer.llvm.org/;) + LandingPage + "#" + - getCheckerFullName()) + std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower(); + + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); } Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif === --- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif +++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif @@ -451,7 +451,7 @@ "fullDescription": { "text": "Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)" }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#core.CallAndMessage;, + "helpUri": "https://clang.llvm.org/docs/analyzer/checkers.html#core-callandmessage;, "id": "core.CallAndMessage", "name": "core.CallAndMessage" }, @@ -459,7 +459,7 @@ "fullDescription": { "text": "Check for division by zero" }, - "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#core.DivideZero;, + "helpUri":
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. Nice! Comment at: clang/utils/TableGen/ClangSACheckersEmitter.cpp:27 -static std::string getPackageFullName(const Record *R); +static std::string getPackageFullName(const Record *R, StringRef Sep = "."); ~~Do you foresee a change in the default separator? I think an eventual shift away from packages is more likely (see https://discourse.llvm.org/t/analyzer-refactoring-analyzeroptions/50160/5). ~~ ~~I this an unnecessary complexity, but its not a hill I will die on.~~ edit: I just realized that you need this for the url, so never mind. I grave dug a 3 yr old topic on discourse because of it :^) Oops. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121387/new/ https://reviews.llvm.org/D121387 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page
steakhal created this revision. steakhal added reviewers: aaron.ballman, NoQ, Szelethus, whisperity, martong. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a project: All. steakhal requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. AFAIK we should prefer https://clang.llvm.org/docs/analyzer/checkers.html to https://clang-analyzer.llvm.org/{available_checks,alpha_checks}.html This patch will ensure that the doc urls produced by tablegen for the ClangSA, will use the new url. Nothing else will be changed. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121387 Files: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif clang/utils/TableGen/ClangSACheckersEmitter.cpp Index: clang/utils/TableGen/ClangSACheckersEmitter.cpp === --- clang/utils/TableGen/ClangSACheckersEmitter.cpp +++ clang/utils/TableGen/ClangSACheckersEmitter.cpp @@ -24,37 +24,39 @@ // Static Analyzer Checkers Tables generation //===--===// -static std::string getPackageFullName(const Record *R); +static std::string getPackageFullName(const Record *R, StringRef Sep = "."); -static std::string getParentPackageFullName(const Record *R) { - std::string name; +static std::string getParentPackageFullName(const Record *R, +StringRef Sep = ".") { if (DefInit *DI = dyn_cast(R->getValueInit("ParentPackage"))) -name = getPackageFullName(DI->getDef()); - return name; +return getPackageFullName(DI->getDef()); + return ""; } -static std::string getPackageFullName(const Record *R) { - std::string name = getParentPackageFullName(R); - if (!name.empty()) -name += "."; - assert(!R->getValueAsString("PackageName").empty()); - name += R->getValueAsString("PackageName"); - return name; +static std::string getPackageFullName(const Record *R, StringRef Sep) { + std::string ParentPkgName = getParentPackageFullName(R, Sep); + StringRef PackageName = R->getValueAsString("PackageName"); + assert(!PackageName.empty()); + + if (ParentPkgName.empty()) +return PackageName.str(); + return (Twine{ParentPkgName} + Sep + PackageName).str(); } -static std::string getCheckerFullName(const Record *R) { - std::string name = getParentPackageFullName(R); - if (!name.empty()) -name += "."; - assert(!R->getValueAsString("CheckerName").empty()); - name += R->getValueAsString("CheckerName"); - return name; +static std::string getCheckerFullName(const Record *R, StringRef Sep = ".") { + std::string ParentPkgName = getParentPackageFullName(R, Sep); + StringRef CheckerName = R->getValueAsString("CheckerName"); + assert(!CheckerName.empty()); + + if (ParentPkgName.empty()) +return CheckerName.str(); + return (Twine{ParentPkgName} + Sep + CheckerName).str(); } static std::string getStringValue(const Record , StringRef field) { if (StringInit *SI = dyn_cast(R.getValueInit(field))) return std::string(SI->getValue()); - return std::string(); + return ""; } // Calculates the integer value representing the BitsInit object @@ -74,20 +76,19 @@ } static std::string getCheckerDocs(const Record ) { - StringRef LandingPage; - if (BitsInit *BI = R.getValueAsBitsInit("Documentation")) { -uint64_t V = getValueFromBitsInit(BI, R); -if (V == 1) - LandingPage = "available_checks.html"; -else if (V == 2) - LandingPage = "alpha_checks.html"; - } - - if (LandingPage.empty()) + const BitsInit *BI = R.getValueAsBitsInit("Documentation"); + if (!BI) +PrintFatalError(R.getLoc(), +"missing Documentation for " + getCheckerFullName()); + + // Ignore 'Documentation' checkers. + if (getValueFromBitsInit(BI, R) == 0) return ""; - return (llvm::Twine("https://clang-analyzer.llvm.org/;) + LandingPage + "#" + - getCheckerFullName()) + std::string CheckerFullName = StringRef(getCheckerFullName(, "-")).lower(); + + return (llvm::Twine("https://clang.llvm.org/docs/analyzer/checkers.html#;) + + CheckerFullName) .str(); } Index: clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif === --- clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif +++ clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif @@ -451,7 +451,7 @@ "fullDescription": { "text": "Check for logical errors for function calls and Objective-C message expressions (e.g., uninitialized arguments, null function pointers)" }, - "helpUri":