=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com>,
=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com>,
=?utf-8?q?Donát?= Nagy <donat.n...@ericsson.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/132...@github.com>
================
@@ -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}};
----------------
steakhal wrote:
I wonder if you could hoist stuff like this to make these fit in 1 line each.
```c++
static constexpr llvm::StringLiteral Desc = "Nullability";
static constexpr llvm::StringLiteral Cat = categories::MemoryError;
BugType BugTypes[NumCheckerParts] = {
{this, NullPassedToNonnullChecker, Desc, Cat},
{this, NullReturnedFromNonnullChecker, Desc, Cat},
{this, NullableDereferencedChecker, Desc, Cat},
{this, NullablePassedToNonnullChecker, Desc, Cat},
{this, NullableReturnedFromNonnullChecker, Desc, Cat},
};
const auto &NullPassedToNonnullBug = BugTypes[NullPassedToNonnullChecker];
const auto &NullReturnedFromNonnullBug =
BugTypes[NullReturnedFromNonnullChecker];
const auto &NullableDereferencedBug = BugTypes[NullableDereferencedChecker];
const auto &NullablePassedToNonnullBug =
BugTypes[NullablePassedToNonnullChecker];
const auto &NullableReturnedFromNonnullBug =
BugTypes[NullableReturnedFromNonnullChecker];
```
This way later you could just directly refer to `NullPassedToNonnullBug`.
---
Every time I see this CheckerPart enum, I question myself, why do we need this?
And I can't convince myself that this is the right way.
Correct me if I'm wrong, but we don't really need a BugType array. We also
don't really need a checker part enum either.
We could design a system that is less coupled. Something like this:
```c++
// Assuming we have a strong-type something like this: struct CheckerPart {};
static inline CheckerPart NullPassedToNonnullChecker;
static inline CheckerPart NullReturnedFromNonnullChecker;
static inline CheckerPart NullableDereferencedChecker;
const BugType NullPassedToNonnullBug{this, NullPassedToNonnullChecker, Desc,
Cat},
// more bug types.
```
The address of the CheckerPart object identifies the "part". The "CheckerPart"
could contain a mutable bool flag to know if it's enabled or not.
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
