[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

This revision was automatically updated to reflect the committed changes. Closed by commit rG6cff2e9f7884: [Analyzer] Bugfix for CheckerRegistry (authored by baloghadamsoftware). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75842/new/ https://revie

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus accepted this revision. Szelethus added a comment. This revision is now accepted and ready to land. LGTM, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75842/new/ https://reviews.llvm.org/D75842 ___ cfe-commits mailing lis

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

baloghadamsoftware updated this revision to Diff 250860. baloghadamsoftware added a comment. Herald added a subscriber: ASDenysPetrov. Updated according to the comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75842/new/ https://reviews.llvm.org/D75842 Files: clang/include/clan

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added inline comments. Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:143 + std::string Diags; + EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags)); +} baloghadamsoftware wrote: > Szelethus wrote: > > I don't think this is

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

baloghadamsoftware marked an inline comment as done. baloghadamsoftware added inline comments. Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:143 + std::string Diags; + EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags)); +} Szelethu

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added inline comments. Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:85 +//===--===// +// Unfulfilled dependency +//===---

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

baloghadamsoftware updated this revision to Diff 249187. baloghadamsoftware added a comment. Test added. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75842/new/ https://reviews.llvm.org/D75842 Files: clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h clang/unittests/Sta

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added a comment. The thing to note here is checkers that are not `isDisabled()` won't get enabled, only if they are `isEnabled()` as well :) Please add a unit test, otherwise LGTM. I just landed D67335 , so it should be easy to do. Repository: rG L

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

xazax.hun added a comment. In D75842#1912364 , @baloghadamsoftware wrote: > In D75842#1912293 , @xazax.hun wrote: > > > I am not sure if I would call this a bugfix. Enabling a checker without one > > of its depend

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

baloghadamsoftware added a comment. In D75842#1912293 , @xazax.hun wrote: > I am not sure if I would call this a bugfix. Enabling a checker without one > of its dependency will potentially cause the checker to misbehave. I am > uncomfortable changing the

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

xazax.hun added a comment. I am not sure if I would call this a bugfix. Enabling a checker without one of its dependency will potentially cause the checker to misbehave. I am uncomfortable changing the current behavior to a more dangerous one without any diagnostics. Including the diagnostic wi

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

Szelethus added a subscriber: dcoughlin. Szelethus added a comment. In D75842#1912249 , @baloghadamsoftware wrote: > In D75842#1912246 , @xazax.hun wrote: > > > If we disable the dependency of a checker explicitly

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

xazax.hun added a comment. If we disable the dependency of a checker explicitly I think we should at least emit a warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75842/new/ https://reviews.llvm.org/D75842

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

baloghadamsoftware added a comment. I looked in the `unittests` directory but could not find tests for dependency handling. @Szelethus, feel free to commandeer this patch if you know how to test it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

baloghadamsoftware added a comment. In D75842#1912246 , @xazax.hun wrote: > If we disable the dependency of a checker explicitly I think we should at > least emit a warning. I agree, but I do not thik that this should be part of this patch. This is only

[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

baloghadamsoftware created this revision. baloghadamsoftware added reviewers: NoQ, Szelethus. baloghadamsoftware added a project: clang. Herald added subscribers: martong, steakhal, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, xazax.hun, whisperity. `Chec