[PATCH] D57855: [analyzer] Reimplement checker options

baloghadamsoftware accepted this revision. baloghadamsoftware added a comment. This revision is now accepted and ready to land. Aside from little comment it seems OK. Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:200 +#undef PACKAGE_OPTION +#undef

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 190977. Szelethus edited the summary of this revision. Szelethus added a comment. - Moved every non-related change to smaller patches, this should ease **a lot** on reviewers. - Now processing options once all checkers are added to the registry. This is

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus planned changes to this revision. Szelethus marked 3 inline comments as done. Szelethus added a comment. Alright, I'll de-clutter this patch a bit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new/ https://reviews.llvm.org/D57855

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus marked 3 inline comments as done. Szelethus added a comment. > Let's put at least a FIXME here that the documentation for this option was > missing. I'd prefer to just simply fix this. @NoQ, could you help us fill in the gaps? We need a desctiption for -

[PATCH] D57855: [analyzer] Reimplement checker options

whisperity added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:49 + "NoDiagnoseCallsToSystemHeaders", + "", + "false"> Let's put at least a FIXME here that the documentation for

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus added inline comments. Comment at: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h:156 + return FullName == Rhs.FullName; +} + baloghadamsoftware wrote: > Are we able now to distinguish checkers of the same short names but in >

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 190223. Szelethus marked 13 inline comments as done. Szelethus added a comment. Fixes according to reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new/ https://reviews.llvm.org/D57855 Files:

[PATCH] D57855: [analyzer] Reimplement checker options

baloghadamsoftware requested changes to this revision. baloghadamsoftware added inline comments. This revision now requires changes to proceed. Comment at: include/clang/StaticAnalyzer/Checkers/CheckerBase.td:49 + string PackageName = name; + list PackageOptions;

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 190022. Szelethus marked an inline comment as done. Szelethus added a comment. Adding `explicit` to `CheckerInfo`'s constructor. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new/ https://reviews.llvm.org/D57855 Files:

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus marked 2 inline comments as done. Szelethus added inline comments. Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:371 +// We can't get away with binaryFind here, as it uses lower_bound. +auto CheckerIt = llvm::find(Checkers,

[PATCH] D57855: [analyzer] Reimplement checker options

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits, but you should wait for someone more well-versed in the static analyzer to sign off before committing. Comment at:

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 189105. Szelethus added a comment. Remembered that we can't use use `llvm::binary_search`, as we'll need the iterators in a later patch. Should've rebased all my branches before updating :^) CHANGES SINCE LAST ACTION

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 189090. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new/ https://reviews.llvm.org/D57855 Files: include/clang/Basic/DiagnosticCommonKinds.td include/clang/StaticAnalyzer/Checkers/CheckerBase.td

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 189088. Szelethus added a comment. Herald added a subscriber: Charusso. - Capitalized some variable names - Added more comments - Preferring binary searches to linear searches wherever possible CHANGES SINCE LAST ACTION

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus added a comment. I guess removing some of the checker option descriptions from `lib/StataicAnalyzer/Checkers/` would be nice too, to easy on maintenance. One could always just use `-analyzer-checker-option-help` whenever in doubt. CHANGES SINCE LAST ACTION

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus marked an inline comment as done. Szelethus added a comment. Since my changes are very invasive towards `CheckerRegistry` in general, I might `clang-format` the whole thing, and make the code follow the coding guidelines. Thanks for the review! CHANGES SINCE LAST ACTION

[PATCH] D57855: [analyzer] Reimplement checker options

aaron.ballman added inline comments. Herald added a subscriber: jdoerfert. Comment at: include/clang/StaticAnalyzer/Checkers/CheckerBase.td:13-14 +/// Describes a checker or package option type. This is important for validating +/// user supplied inputs. +class

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus marked 5 inline comments as done. Szelethus added inline comments. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:384 + CheckerOptions<[ +CmdLineOption It would be great if there were a way to define options once and reuse them. I think it's

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 186350. Szelethus added a comment. - Removed the accidentally added `nullability:Optimistic` entry, - Reworded `unix.DynamicMemoryModeling:Optimistic`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new/ https://reviews.llvm.org/D57855

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus marked an inline comment as done. Szelethus added a comment. In D57855#1392560 , @xazax.hun wrote: > We have `examples/analyzer-plugin`. I would prefer to add an example option > to the example plugin so people do see how to do this when they

[PATCH] D57855: [analyzer] Reimplement checker options

xazax.hun added a comment. We have `examples/analyzer-plugin`. I would prefer to add an example option to the example plugin so people do see how to do this when they are registering a checker from a plugin. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:48 +

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus added a reviewer: aaron.ballman. Szelethus added a comment. Aaron, you recently changed these file too, if you have the time, I'll happily listen to your feedback :) Also, I was thinking that maybe we could just `std::move` `CheckerRegisrty::Checkers` and `CheckerRegisrty::Packages`

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus updated this revision to Diff 185861. Szelethus edited the summary of this revision. Szelethus added a comment. Moved all `AnalyzerOptions` related changes to a separate revision. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57855/new/ https://reviews.llvm.org/D57855 Files:

[PATCH] D57855: [analyzer] Reimplement checker options

Szelethus created this revision. Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, baloghadamsoftware. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, mgrang, szepet, whisperity. Herald added a project: clang. Here we go