aaron.ballman added a comment.
In https://reviews.llvm.org/D33672#819683, @xazax.hun wrote:
> Aaron, could you comment on the applicability of this check to C? Thanks in
> advance.
C has different rules for their enumerations in that the enumerators are all
ints, but the enumeration type is either `char`, a signed integer type, or an
unsigned integer type depending on the values of the enumerators. So this
problem exists:
enum E {
one = 1
};
void f(int i) {
enum E e = (enum E)i;
}
int main(void) {
f(1024);
}
If `enum E` is represented by a `char`, then the cast causes a loss of
precision. However, this isn't undefined behavior in C like it is in C++.
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:29
+namespace {
+// This evaluator checks 2 SVals for equality. The first SVal is provided via
+// the constructor, the second is the parameter of the overloaded () operator.
----------------
s/2/two
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:34
+class ConstraintBasedEQEvaluator {
+private:
+ const DefinedOrUnknownSVal CompareValue;
----------------
No need for the access specifier; it defaults to `private`.
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:47
+ DefinedOrUnknownSVal EnumDeclValue = SVB.makeIntVal(EnumDeclInitValue);
+ const auto ElemEqualsValueToCast =
+ SVB.evalEQ(PS, EnumDeclValue, CompareValue);
----------------
Please do not use `auto` here as the type is not spelled out in the
initialization. Also, you can drop the `const` qualifier if it's not a pointer
or reference type.
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:74-75
+ EnumValueVector DeclValues;
+ for (const auto *D : ED->decls()) {
+ const auto ECD = dyn_cast<EnumConstantDecl>(D);
+ DeclValues.push_back(ECD->getInitVal());
----------------
Instead of enumerating over `decls()` and then casting, just enumerate over
`enumerators()` and the cast isn't needed. Or, even better:
```
EnumValueVector DeclValues(ED->enumerator_end() - ED->enumerator_begin());
std::transform(ED->enumerator_begin(), ED->enumerator_end(), DeclValues.begin(),
[](const EnumConstantDecl *D) { return D->getInitVal();
});
```
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:84
+void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C) const {
+ if (auto N = C.generateNonFatalErrorNode(C.getState())) {
+ if (!EnumValueCastOutOfRange)
----------------
NoQ wrote:
> `C.getState()` is the default value (if you see how
> `generateNonFatalErrorNode()` calls `addTransition()` which in turns
> substitutes nullptr with `getState()`), so it can be omitted.
Don't use `auto` here.
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:87-89
+ new BuiltinBug(this, "Enum cast out of range",
+ "The value provided to the cast expression is not in "
+ "the valid range of values for the enum."));
----------------
Also, diagnostics should not be full sentences or grammatically correct, so
drop the capitalization and full stop.
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:116
+ // function to handle this.
+ const EnumDecl *ED = T->getAs<EnumType>()->getDecl();
+
----------------
You can use `castAs<>()` because you've already determined it's an enumeral
type.
================
Comment at: lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121
+ bool PossibleValueMatch =
+ std::any_of(DeclValues.begin(), DeclValues.end(),
+ ConstraintBasedEQEvaluator(C, *ValueToCastOptional));
----------------
You can use `llvm::any_of()` and pass in the container.
https://reviews.llvm.org/D33672
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
