[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Good catch!



================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104
+    const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+      const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+      if (const auto *ET = dyn_cast<EnumType>(CanonicalType))
+        return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
+      return Ty->isIntegralOrEnumerationType();
+    };
----------------
I don't really see any reasons why is this a lambda and not a free function


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:101
+      if (const auto *ET = dyn_cast<EnumType>(CanonicalType))
+        return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
----------------
I think it's better to add tests for this condition.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:106-107
+
     // FIXME: Remove the second disjunct when we support symbolic
     // truncation/extension.
+    const bool BothHaveSameCanonicalTypes =
----------------
Maybe this comment should be moved closer to the `return` statement?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85528