================ @@ -112,25 +112,30 @@ class NullabilityChecker void printState(raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep) const override; - enum CheckKind { - CK_NullPassedToNonnull, - CK_NullReturnedFromNonnull, - CK_NullableDereferenced, - CK_NullablePassedToNonnull, - CK_NullableReturnedFromNonnull, - CK_NumCheckKinds + // FIXME: This enumeration of checker parts is extremely similar to the + // ErrorKind enum. It would be nice to unify them to simplify the code. + enum : CheckerPartIdx { + NullPassedToNonnullChecker, + NullReturnedFromNonnullChecker, + NullableDereferencedChecker, + NullablePassedToNonnullChecker, + NullableReturnedFromNonnullChecker, + NumCheckerParts }; - bool ChecksEnabled[CK_NumCheckKinds] = {false}; - CheckerNameRef CheckNames[CK_NumCheckKinds]; - mutable std::unique_ptr<BugType> BTs[CK_NumCheckKinds]; - - const std::unique_ptr<BugType> &getBugType(CheckKind Kind) const { - if (!BTs[Kind]) - BTs[Kind].reset(new BugType(CheckNames[Kind], "Nullability", - categories::MemoryError)); - return BTs[Kind]; - } + // FIXME: Currently the `Description` fields of these `BugType`s are all + // identical ("Nullability") -- they should be more descriptive than this. + BugType BugTypes[NumCheckerParts] = { + {this, NullPassedToNonnullChecker, "Nullability", + categories::MemoryError}, + {this, NullReturnedFromNonnullChecker, "Nullability", + categories::MemoryError}, + {this, NullableDereferencedChecker, "Nullability", + categories::MemoryError}, + {this, NullablePassedToNonnullChecker, "Nullability", + categories::MemoryError}, + {this, NullableReturnedFromNonnullChecker, "Nullability", + categories::MemoryError}}; ---------------- NagyDonat wrote:
> Correct me if I'm wrong, but we don't really need a BugType array. Yep, you're completely right, and I should have noticed this -- e.g. `DivZeroChecker` also declares the bug types as individual variables (and not arrays). > We also don't really need a checker part enum either. [...] When I designed the current framework I implicitly assumed that we will need a checker part _identifier_ type which acts as an index/key for a (perhaps associative) array containing the names / registration status, because this was a common element in all the individual checkers plus the old abandoned `SubChecker` draft commit that was written by Kristóf Umann several years ago. (I spent lots of time on thinking about details _within this box_, but I didn't consider out-of-the box ideas.) Now that I think about your suggestion, I see that it could lead to a simpler alternative framework. The operations that a checker part type needs to support: - The constructor of `BugType` is called from the constructor of a checker and needs to take a checker part argument (so the checker part should be available at this point). - `BugType` stores this checker part as a data member (so all checkers should use the same `CheckerPart` type). - `CheckerManager::registerChecker` takes a checker part as an argument (in fact, currently as a template argument, but this may change) and binds the name injected from `Checkers.td` to it. (Binding the name implicitly marks the part as enabled, because enabled parts need to have names, while others don't have names.) - The enabled/disabled status of a checker part may be queried within various checker callbacks. - The name of the checker part is queried by the bug types associated with it and may be queried elsewhere within the checker callbacks. - `getTagDescription()` currently iterates over the checker parts to find one that has a name. This is the only case where _iteration over checker parts_ happen, so perhaps it should be replaced by an alternative implementation. These could be satisfied by a checker part type that looks like ```c++ class CheckerPart { std::optional<CheckerNameRef> Name; public: void enable(CheckerManager &Mgr) { assert(!Name && "Checker part registered twice!"); Name = Mgr.getCurrentCheckerName(); } bool isEnabled() { return static_cast<bool>(Name); } CheckerNameRef getName() { return *Name; } } class FooOrBarChecker { public: CheckerPart Foo, Bar; private: BugType FooBug{Foo, "Foo does not work"}; // ... } void registerFooChecker(CheckerManager &Mgr) { Mgr.registerChecker<FooOrBarChecker>()->Foo.enable(Mgr); } ``` https://github.com/llvm/llvm-project/pull/132250 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits