[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 18 08:19:09 PST 2018


aaron.ballman added a comment.

In D55792#1334644 <https://reviews.llvm.org/D55792#1334644>, @Szelethus wrote:

> Global nit: I guess having both DESC and DOCS as variable/macro arg names is confusing. Can we instead use DOCSURI/DocsUri?


Sure, I'll make that change.

> In D55792#1334339 <https://reviews.llvm.org/D55792#1334339>, @aaron.ballman wrote:
> 
>> In D55792#1334291 <https://reviews.llvm.org/D55792#1334291>, @Szelethus wrote:
>>
>> > Thank you so much for working on this! The lack of a 
>> >  I guess it'd be better to generate the entire URI in the tblgen file, and also allow plugins to register their own through `CheckerRegistry`. Sarif lacking the support for them is one thing, but adding a new field to the `Checker` class in `CheckerBase.td` I think should be accompanied by extending `CheckerRegistry::CheckerInfo` and `CheckerRegistry::getChecker`.
>>
>>
>> Good point about the checker registry, I'll make those changes. However, I'm a bit less certain about generating the entire URI in the tablegen file -- that adds a lot of string literals to the binary when I think we don't have to. However, perhaps that's not a huge concern and having the information in its final form in the .inc file is a better approach. WDYT?
> 
> 
> Well, we already store checker- and the quite lengthy -analyzer-config descriptions... but I'm not sure thats a good point.
>  However, plugin checkers being able to specify uris such as "mycompany.internal.supersecret.idk/dontshare/checkers/tradesecrets.html" would be neat.
> 
> One more thing to consider is that if Sphinx lands after 8.0.0., the uri will most probably change. We can always redirect, but thats a thing to consider. I'm not sure whether there's any likelihood of it being completed until the new release branch is created. But I don't think there's any change needed for this patch just because of this.

Yeah, I wound up pushing all of the data into the table generator as suggested. The plugin registration was what did it for me -- when registering a checker, we should have the full link to the help URI rather than try to build it from pieces that may not be correct for plugins.


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

https://reviews.llvm.org/D55792





More information about the cfe-commits mailing list