NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:183
+ if (Body)
+ DRE = dyn_cast<DeclRefExpr>(Body);
+
----------------
A body of a function is always either a `CompoundStmt` or (shockingly) a
`CXXTryStmt`. This cast will always fail.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:188
+
+ QualType CastToTy = DRE->getTemplateArgs()->getArgument().getAsType();
+ QualType CastFromTy = getRecordType(Call.parameters()[0]->getType());
----------------
I suspect that `DRE` may still be null (eg., when calling `isa` through
function pointer).
I think you should just lookup `Call.getDecl()`'s template arguments instead.
It's going to be the declaration of the specific instantiation, so it'll have
all the arguments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:216
+ const NoteTag *Tag = C.getNoteTag(
+ [=](BugReport &) -> std::string {
+ SmallString<128> Msg;
----------------
Can you use the overload without the bug report parameter? Cause you don't seem
to be using it anyway.
================
Comment at: clang/test/Analysis/cast-value-notes.cpp:5-27
namespace llvm {
template <class X, class Y>
const X *cast(Y Value);
template <class X, class Y>
const X *dyn_cast(Y *Value);
template <class X, class Y>
----------------
Szelethus wrote:
> Hmm, we may want to move these eventually to `clang/test/Inputs/`, since this
> isn't the only LLVM checker. Thought that said, I want to see the other LLVM
> checker gone, like, purged from the codebase for being the abomination that
> it is.
>
> Feel free to leave these here for know, just thinking aloud :)
I agree, extracting common definitions into a system header simulator usually
ends up being worth it pretty quickly.
================
Comment at: clang/test/Analysis/cast-value-notes.cpp:89
+ if (isa<Triangle>(C)) {
+ // expected-note@-1 {{'C' with type 'Circle' is not the instance of
'Triangle'}}
+ // expected-note@-2 {{Taking false branch}}
----------------
I suggest: `'C' is a 'Circle', not a 'Triangle'`.
================
Comment at: clang/test/Analysis/cast-value-notes.cpp:95
+ if (isa<Circle>(C)) {
+ // expected-note@-1 {{'C' with type 'Circle' is a 'Circle'}}
+ // expected-note@-2 {{Taking true branch}}
----------------
Just `'C' is a 'Circle'`.
================
Comment at: clang/test/Analysis/cast-value-notes.cpp:111
+ if (isa<Triangle>(C)) {
+ // expected-note@-1 {{Assuming 'C' with type 'Circle' is not the instance
of 'Triangle'}}
+ // expected-note@-2 {{Taking false branch}}
----------------
This test looks incorrect. We shouldn't be assuming this, we already know that
it's not a triangle because we know that it's a circle.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66423/new/
https://reviews.llvm.org/D66423
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
