steakhal added a comment.
This patch preserves all previous reports as expected.
You can check it by yourself at
https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs?run=D96090&items-per-page=50.
However, I still have some minor concerns inline.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:101-103
// FIXME: Make these protected again once RegionStoreManager correctly
// handles loads from different bound value types.
virtual SVal dispatchCast(SVal val, QualType castTy) = 0;
----------------
I'm not sure if this FIXME is still applicable.
I'm also confused about having two functions doing effectively the same thing.
`SimpleSValBuilder::dispatchCast` is a virtual function, which just invokes a
non-virtual function `SValBuilder::evalCast`.
Why should it be virtual in the first place?
================
Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:727-728
+ // pointers as well.
+ // FIXME: We really need a single good function to perform casts for us
+ // correctly every time we need it.
+ if (CastTy->isPointerType() && !CastTy->isVoidPointerType()) {
----------------
You are resolving exactly this FIXME in this patch if I'm correct.
Shouldn't you update this comment?
================
Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:868-870
+ if (OriginalTy.isNull())
+ // Pass to MemRegion function.
+ return evalCastSubKind(loc::MemRegionVal(R), CastTy, OriginalTy);
----------------
Eh, I don't like comments preceding a single-statement block.
It might be a personal preference though.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96090/new/
https://reviews.llvm.org/D96090
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
