[PATCH] D90157: [analyzer] Rework SValBuilder::evalCast function into maintainable and clear way

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:647-650
+  // Array to pointer.
+  if (isa<ArrayType>(OriginalTy))
+    if (CastTy->isPointerType() || CastTy->isReferenceType())
       return UnknownVal();
----------------
ASDenysPetrov wrote:
> steakhal wrote:
> > Arrays decay to a pointer to the first element, but in this case, you return `Unknown`.
> > Why?
> This is just how `evalCast` worked before the patch. I didn't think of why. I've just tried to replicate previous cast logic. If I'd change anything, you'd catch a bunch of differences in CodeChecker. That's what I didn't want the most.
> 
I agree, you should not change any behavior in this patch.


================
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;
+  };
----------------
ASDenysPetrov wrote:
> steakhal wrote:
> > Just call immediately that lambda and assign that value to a `const llvm::APSInt CastedValue`.
> I just wanted to make this lazy. Otherwise this set of calls will be invoked unnecessarily in some cases. I'd prefer to leave it as it is.
Maybe rename it then. What about `GetCastedValue(x)`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90157/new/

https://reviews.llvm.org/D90157