steakhal added a comment.
What you are basically doing is a visitation on the kind of the `SVal`.
Why don't you use the `SValVisitor` instead? That way you could focus on the
leaf nodes in the hierarchy, leaving you an even cleaner solution.
class CastEvaluator : public SValVisitor<CastEvaluator, SVal> {
public:
CastEvaluator(QualType CastTy, QualType OriginalTy /*etc.*/)
SVal VisitPointerToMember(nonloc::PointerToMember V) { return V; }
SVal VisitGotoLabel(loc::GotoLabel V) { /*impl.*/ }
/* etc. */
};
Comments should be punctuated and the variables should start with an uppercase
letter in general.
About the correctness: I think it still needs a bit more confidence. I'm gonna
run this on LLVM itself and see if anything changes.
I really like where this is going. Keep up the good work.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:105-126
+ SVal evalCast(SVal V, QualType CastTy, QualType OriginalTy);
+ SVal evalCastKind(UndefinedVal V, QualType CastTy, QualType OriginalTy);
+ SVal evalCastKind(UnknownVal V, QualType CastTy, QualType OriginalTy);
+ SVal evalCastKind(Loc V, QualType CastTy, QualType OriginalTy);
+ SVal evalCastKind(NonLoc V, QualType CastTy, QualType OriginalTy);
+ SVal evalCastSubKind(loc::ConcreteInt V, QualType CastTy,
+ QualType OriginalTy);
----------------
Why are all of these `public`?
I would expect only the first overload to be public.
================
Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:647-650
+ // Array to pointer.
+ if (isa<ArrayType>(OriginalTy))
+ if (CastTy->isPointerType() || CastTy->isReferenceType())
return UnknownVal();
----------------
Arrays decay to a pointer to the first element, but in this case, you return
`Unknown`.
Why?
================
Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:680-681
+
+ // Try to cast to array
+ const auto *arrayT = dyn_cast<ArrayType>(OriginalTy.getCanonicalType());
+
----------------
nit
================
Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:765-769
+ auto castedValue = [V, CastTy, this]() {
+ llvm::APSInt value = V.getValue();
+ BasicVals.getAPSIntType(CastTy).apply(value);
+ return value;
+ };
----------------
Just call immediately that lambda and assign that value to a `const
llvm::APSInt CastedValue`.
================
Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:844-847
+ // Symbol to bool.
+ if (CastTy->isBooleanType()) {
+ // Non-float(presumably) to bool.
+ if (Loc::isLocType(OriginalTy) ||
----------------
Presumably what?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90157/new/
https://reviews.llvm.org/D90157
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
