[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting
Kristóf Umann via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 24 04:37:13 PDT 2020
Szelethus added a comment.
I personally preferred the previous diff, the reporting part of the checker and the string length modeling was separated far more cleanly.
In D84316#2171267 <https://reviews.llvm.org/D84316#2171267>, @NoQ wrote:
> Mmm, none of these benefits sound like they outweigh confusing the cost of users with a new checker flag that can't even be used in any sensible way.
I think the new checker would be an ideal candidate for a hidden checker, not to mention that D78126 <https://reviews.llvm.org/D78126>+D81761 <https://reviews.llvm.org/D81761> enforces it anyways with asserts. With that in mind, I don't see what we're giving up here.
> If you want separate files, just put the checker into a header and include it from multiple cpp files. A few checkers already do that - RetainCountChecker, MPIChecker, UninitializedObjectChecker. There's nothing fundamental about keeping checkers in an anonymous namespace. With that said, I don't want to make an example out of RetainCountChecker -- the way it is structured is not in line, at least the way I see it, with the modern way to develop large checkers.
I agree that there is no magical gain from keeping them there. UninitializedObjectChecker, btw, doesn't peek out -- only some of the related infrastructure.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84316/new/
https://reviews.llvm.org/D84316
More information about the cfe-commits
mailing list