NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51
+
+ const CallDescriptionMap<CastCheck> SugarCastCDM = {
+ {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs},
----------------
I'd rather have only one map from call descriptions to (callback, kind) pairs.
This should make the `evalCall` code much more readable.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:215
const auto *CE = cast<CallExpr>(Call.getOriginExpr());
- if (!CE)
+ if (!CE->getType()->getPointeeCXXRecordDecl())
return false;
----------------
Use `Call.getResultType()` instead.
And please add a test with references, as `getResultType` behaves more
correctly for references:
```lang=c++
C &c = ...;
D &d = dyn_cast<D>(c);
```
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:223
+ // If we cannot obtain the arg's class we cannot be sure how to model it.
+ if (!CE->getArg(0)->getType()->getPointeeCXXRecordDecl())
+ return false;
----------------
`*Call.parameters()[0]->getType()`. It should also help with references.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:231
+ // If we cannot obtain 'MCE' we cannot be sure how to model it.
+ const auto *MCE = dyn_cast<CXXMemberCallExpr>(CE->IgnoreParenImpCasts());
+ if (!MCE)
----------------
```lang=c++
const auto *InstanceCall = dyn_cast<CXXInstanceCall>(&Call);
...
DV = InstanceCall->getCXXThisVal();
```
================
Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:718
+ assert(ThisVal.isUnknownOrUndef() || ThisVal.getAs<Loc>() ||
+ ThisVal.getAs<nonloc::ConcreteInt>()->getValue().getExtValue() == 1);
return ThisVal;
----------------
I don't think you need this anymore.
================
Comment at: clang/test/Analysis/cast-value.cpp:186
+ // expected-note@-1 {{Assuming pointer value is null}}
+ // expected-note@-2 {{Assuming dynamic cast to 'Circle' succeeds}}
+ // expected-note@-3 {{'C' initialized here}}
----------------
To think: this note isn't very useful because hard casts aren't really supposed
to fail anyway.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65889/new/
https://reviews.llvm.org/D65889
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
