[clang] [analyzer][NFC] Rework SVal kind representation (PR #71039)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 3 04:51:31 PDT 2023


https://github.com/steakhal updated https://github.com/llvm/llvm-project/pull/71039

>From 3bc43ab005aa76a43644d4d93286215b490cc8fa Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Thu, 2 Nov 2023 10:21:03 +0100
Subject: [PATCH 1/2] [analyzer][NFC] Rework SVal kind representation

The goal of this patch is to refine how the `SVal` base and sub-kinds
are represented by forming one unified enum describing the possible SVals.
This means that the `unsigned SVal::Kind` and the attached bit-packing
semantics would be replaced by a single unified enum.
This is more conventional and leads to a better debugging experience by default.
This eases the need of using debug pretty-printers, or the use of
runtime functions doing the printing for us like we do today by calling
`Val.dump()` whenever we inspect the values.

Previously, the first 2 bits of the `unsigned SVal::Kind` discriminated
the following quartet: `UndefinedVal`, `UnknownVal`, `Loc`, or `NonLoc`.
The rest of the upper bits represented the sub-kind, where the value
represented the index among only the `Loc`s or `NonLoc`s, effectively
attaching 2 meanings of the upper bits depending on the base-kind.
We don't need to pack these bits, as we have plenty even if we would use
just a plan-old `unsigned char`.

Consequently, in this patch, I propose to lay out all the (non-abstract)
`SVal` kinds into a single enum, along with some metadata (`BEGIN_Loc`,
`END_Loc`, `BEGIN_NonLoc`, `END_NonLoc`) artificial enum values, similar
how we do with the `MemRegions`.

Note that in the unified `SVal::Kind` enum, to differentiate
`nonloc::ConcreteInt` from `loc::ConcreteInt`, I had to prefix them with
`Loc` and `NonLoc` to resolve this ambiguity.
This should not surface in general, because I'm replacing the
`nonloc::Kind` enum items with `inline constexpr` global constants to
mimic the original behavior - and offer nicer spelling to these enum
values.

Some `SVal` constructors were not marked explicit, which I now mark as
such to follow best practices, and marked others as `/*implicit*/` to
clarify the intent.
During refactoring, I also found at least one function not marked
`LLVM_ATTRIBUTE_RETURNS_NONNULL`, so I did that.

The `TypeRetrievingVisitor` visitor had some accidental dead code,
namely: `VisitNonLocConcreteInt` and `VisitLocConcreteInt`.

Previously, the `SValVisitor` expected visit handlers of
`VisitNonLocXXXXX(nonloc::XXXXX)` and `VisitLocXXXXX(loc::XXXXX)`, where
I felt that envoding `NonLoc` and `Loc` in the name is not necessary as
the type of the parameter would select the right overload anyways, so I
simplified the naming of those visit functions.

The rest of the diff is a lot of times just formatting, because
`getKind()` by nature, frequently appears in switches, which means that
the whole switch gets automatically reformatted. I could probably undo
the formatting, but I didn't want to deviate from the rule unless
explicitly requested.
---
 .../StaticAnalyzer/Checkers/SValExplainer.h   |  10 +-
 .../Core/PathSensitive/SValVisitor.h          |  57 ++---
 .../Core/PathSensitive/SVals.def              |  38 ++-
 .../StaticAnalyzer/Core/PathSensitive/SVals.h | 225 ++++++------------
 clang/lib/StaticAnalyzer/Core/SValBuilder.cpp |  28 +--
 clang/lib/StaticAnalyzer/Core/SVals.cpp       |  87 ++++---
 .../Core/SimpleConstraintManager.cpp          |   4 +-
 .../StaticAnalyzer/Core/SimpleSValBuilder.cpp |  73 +++---
 clang/lib/StaticAnalyzer/Core/Store.cpp       |   2 +-
 9 files changed, 220 insertions(+), 304 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h b/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
index 3ae28c1dba3eb5a..43a70f596a4da28 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
@@ -65,7 +65,7 @@ class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
     return "undefined value";
   }
 
-  std::string VisitLocMemRegionVal(loc::MemRegionVal V) {
+  std::string VisitMemRegionVal(loc::MemRegionVal V) {
     const MemRegion *R = V.getRegion();
     // Avoid the weird "pointer to pointee of ...".
     if (auto SR = dyn_cast<SymbolicRegion>(R)) {
@@ -76,7 +76,7 @@ class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
     return "pointer to " + Visit(R);
   }
 
-  std::string VisitLocConcreteInt(loc::ConcreteInt V) {
+  std::string VisitConcreteInt(loc::ConcreteInt V) {
     const llvm::APSInt &I = V.getValue();
     std::string Str;
     llvm::raw_string_ostream OS(Str);
@@ -84,11 +84,11 @@ class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
     return Str;
   }
 
-  std::string VisitNonLocSymbolVal(nonloc::SymbolVal V) {
+  std::string VisitSymbolVal(nonloc::SymbolVal V) {
     return Visit(V.getSymbol());
   }
 
-  std::string VisitNonLocConcreteInt(nonloc::ConcreteInt V) {
+  std::string VisitConcreteInt(nonloc::ConcreteInt V) {
     const llvm::APSInt &I = V.getValue();
     std::string Str;
     llvm::raw_string_ostream OS(Str);
@@ -97,7 +97,7 @@ class SValExplainer : public FullSValVisitor<SValExplainer, std::string> {
     return Str;
   }
 
-  std::string VisitNonLocLazyCompoundVal(nonloc::LazyCompoundVal V) {
+  std::string VisitLazyCompoundVal(nonloc::LazyCompoundVal V) {
     return "lazily frozen compound value of " + Visit(V.getRegion());
   }
 
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h
index fc83e26183b3e7d..f7bb37f6591671f 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h
@@ -14,9 +14,9 @@
 #ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALVISITOR_H
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALVISITOR_H
 
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 
 namespace clang {
 
@@ -25,49 +25,42 @@ namespace ento {
 /// SValVisitor - this class implements a simple visitor for SVal
 /// subclasses.
 template <typename ImplClass, typename RetTy = void> class SValVisitor {
-public:
-
-#define DISPATCH(NAME, CLASS) \
-  return static_cast<ImplClass *>(this)->Visit ## NAME(V.castAs<CLASS>())
+  ImplClass &derived() { return *static_cast<ImplClass *>(this); }
 
+public:
   RetTy Visit(SVal V) {
     // Dispatch to VisitFooVal for each FooVal.
-    // Take namespaces (loc:: and nonloc::) into account.
-    switch (V.getBaseKind()) {
-#define BASIC_SVAL(Id, Parent) case SVal::Id ## Kind: DISPATCH(Id, Id);
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
-    case SVal::LocKind:
-      switch (V.getSubKind()) {
-#define LOC_SVAL(Id, Parent) \
-      case loc::Id ## Kind: DISPATCH(Loc ## Id, loc :: Id);
+    switch (V.getKind()) {
+#define BASIC_SVAL(Id, Parent)                                                 \
+  case SVal::Id##Kind:                                                         \
+    return derived().Visit##Id(V.castAs<Id>());
+#define LOC_SVAL(Id, Parent)                                                   \
+  case SVal::Loc##Id##Kind:                                                    \
+    return derived().Visit##Id(V.castAs<loc::Id>());
+#define NONLOC_SVAL(Id, Parent)                                                \
+  case SVal::NonLoc##Id##Kind:                                                 \
+    return derived().Visit##Id(V.castAs<nonloc::Id>());
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
-      }
-      llvm_unreachable("Unknown Loc sub-kind!");
-    case SVal::NonLocKind:
-      switch (V.getSubKind()) {
-#define NONLOC_SVAL(Id, Parent) \
-      case nonloc::Id ## Kind: DISPATCH(NonLoc ## Id, nonloc :: Id);
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
-      }
-      llvm_unreachable("Unknown NonLoc sub-kind!");
     }
     llvm_unreachable("Unknown SVal kind!");
   }
 
-#define BASIC_SVAL(Id, Parent) \
-  RetTy Visit ## Id(Id V) { DISPATCH(Parent, Id); }
-#define ABSTRACT_SVAL(Id, Parent) \
-  BASIC_SVAL(Id, Parent)
-#define LOC_SVAL(Id, Parent) \
-  RetTy VisitLoc ## Id(loc::Id V) { DISPATCH(Parent, Parent); }
-#define NONLOC_SVAL(Id, Parent) \
-  RetTy VisitNonLoc ## Id(nonloc::Id V) { DISPATCH(Parent, Parent); }
+  // Dispatch to the more generic handler as a default implementation.
+#define BASIC_SVAL(Id, Parent)                                                 \
+  RetTy Visit##Id(Id V) { return derived().Visit##Parent(V.castAs<Id>()); }
+#define ABSTRACT_SVAL(Id, Parent) BASIC_SVAL(Id, Parent)
+#define LOC_SVAL(Id, Parent)                                                   \
+  RetTy Visit##Id(loc::Id V) {                                                 \
+    return derived().Visit##Parent(V.castAs<Parent>());                        \
+  }
+#define NONLOC_SVAL(Id, Parent)                                                \
+  RetTy Visit##Id(nonloc::Id V) {                                              \
+    return derived().Visit##Parent(V.castAs<Parent>());                        \
+  }
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
 
   // Base case, ignore it. :)
   RetTy VisitSVal(SVal V) { return RetTy(); }
-
-#undef DISPATCH
 };
 
 /// SymExprVisitor - this class implements a simple visitor for SymExpr
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
index eb05de6d9933342..36d2425d155a929 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
@@ -6,28 +6,24 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// The list of symbolic values (SVal kinds and sub-kinds) used in the Static
-// Analyzer. The distinction between loc:: and nonloc:: SVal namespaces is
+// The list of symbolic values (SVal kinds) used in the Static Analyzer.
+// The distinction between `loc::` and `nonloc::` SVal namespaces is
 // currently hardcoded, because it is too peculiar and explicit to be handled
 // uniformly. In order to use this information, users of this file must define
 // one or more of the following macros:
 //
-// BASIC_SVAL(Id, Parent) - for specific SVal sub-kinds, which are
-// neither in loc:: nor in nonloc:: namespace; these classes occupy
-// their own base kind IdKind.
+// BASIC_SVAL(Id, Parent) - for specific SVal kinds, which are
+// neither in `loc::` nor in `nonloc::` namespace.
 //
 // ABSTRACT_SVAL(Id, Parent) - for abstract SVal classes which are
-// neither in loc:: nor in nonloc:: namespace,
+// neither in `loc::` nor in `nonloc::` namespace,
 //
-// ABSTRACT_SVAL_WITH_KIND(Id, Parent) - for SVal classes which are also
-// neither in loc:: nor in nonloc:: namespace, but occupy a whole base kind
-// identifier IdKind, much like BASIC_SVALs.
+// LOC_SVAL(Id, Parent) - for values in `loc::` namespace.
 //
-// LOC_SVAL(Id, Parent) - for values in loc:: namespace, which occupy a sub-kind
-// loc::IdKind.
+// NONLOC_SVAL(Id, Parent) - for values in `nonloc::` namespace.
 //
-// NONLOC_SVAL(Id, Parent) - for values in nonloc:: namespace, which occupy a
-// sub-kind nonloc::IdKind.
+// SVAL_RANGE(Id, First, Last) - for defining range of subtypes of
+// the abstract class `Id`.
 //
 //===----------------------------------------------------------------------===//
 
@@ -39,10 +35,6 @@
 #define ABSTRACT_SVAL(Id, Parent)
 #endif
 
-#ifndef ABSTRACT_SVAL_WITH_KIND
-#define ABSTRACT_SVAL_WITH_KIND(Id, Parent) ABSTRACT_SVAL(Id, Parent)
-#endif
-
 #ifndef LOC_SVAL
 #define LOC_SVAL(Id, Parent)
 #endif
@@ -51,24 +43,30 @@
 #define NONLOC_SVAL(Id, Parent)
 #endif
 
+#ifndef SVAL_RANGE
+#define SVAL_RANGE(Id, First, Last)
+#endif
+
 BASIC_SVAL(UndefinedVal, SVal)
 ABSTRACT_SVAL(DefinedOrUnknownSVal, SVal)
   BASIC_SVAL(UnknownVal, DefinedOrUnknownSVal)
   ABSTRACT_SVAL(DefinedSVal, DefinedOrUnknownSVal)
-    ABSTRACT_SVAL_WITH_KIND(Loc, DefinedSVal)
+    ABSTRACT_SVAL(Loc, DefinedSVal)
       LOC_SVAL(ConcreteInt, Loc)
       LOC_SVAL(GotoLabel, Loc)
       LOC_SVAL(MemRegionVal, Loc)
-    ABSTRACT_SVAL_WITH_KIND(NonLoc, DefinedSVal)
+      SVAL_RANGE(Loc, ConcreteInt, MemRegionVal)
+    ABSTRACT_SVAL(NonLoc, DefinedSVal)
       NONLOC_SVAL(CompoundVal, NonLoc)
       NONLOC_SVAL(ConcreteInt, NonLoc)
       NONLOC_SVAL(LazyCompoundVal, NonLoc)
       NONLOC_SVAL(LocAsInteger, NonLoc)
       NONLOC_SVAL(SymbolVal, NonLoc)
       NONLOC_SVAL(PointerToMember, NonLoc)
+      SVAL_RANGE(NonLoc, CompoundVal, PointerToMember)
 
+#undef SVAL_RANGE
 #undef NONLOC_SVAL
 #undef LOC_SVAL
-#undef ABSTRACT_SVAL_WITH_KIND
 #undef ABSTRACT_SVAL
 #undef BASIC_SVAL
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index 00cce21151a7073..78239d4060c47d0 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -18,6 +18,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/PointerUnion.h"
@@ -47,50 +48,30 @@ class PointerToMemberData;
 class SValBuilder;
 class TypedValueRegion;
 
-namespace nonloc {
-
-/// Sub-kinds for NonLoc values.
-enum Kind {
-#define NONLOC_SVAL(Id, Parent) Id ## Kind,
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
-};
-
-} // namespace nonloc
-
-namespace loc {
-
-/// Sub-kinds for Loc values.
-enum Kind {
-#define LOC_SVAL(Id, Parent) Id ## Kind,
-#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
-};
-
-} // namespace loc
-
 /// SVal - This represents a symbolic expression, which can be either
 ///  an L-value or an R-value.
 ///
 class SVal {
 public:
-  enum BaseKind {
-    // The enumerators must be representable using 2 bits.
-#define BASIC_SVAL(Id, Parent) Id ## Kind,
-#define ABSTRACT_SVAL_WITH_KIND(Id, Parent) Id ## Kind,
+  enum SValKind : unsigned char {
+#define BASIC_SVAL(Id, Parent) Id##Kind,
+#define LOC_SVAL(Id, Parent) Parent##Id##Kind,
+#define NONLOC_SVAL(Id, Parent) Parent##Id##Kind,
+#define SVAL_RANGE(Id, First, Last)                                            \
+  BEGIN_##Id = Id##First##Kind, END_##Id = Id##Last##Kind,
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
   };
-  enum { BaseBits = 2, BaseMask = 0b11 };
 
 protected:
   const void *Data = nullptr;
+  SValKind Kind = UndefinedValKind;
 
-  /// The lowest 2 bits are a BaseKind (0 -- 3).
-  ///  The higher bits are an unsigned "kind" value.
-  unsigned Kind = 0;
-
-  explicit SVal(const void *d, bool isLoc, unsigned ValKind)
-      : Data(d), Kind((isLoc ? LocKind : NonLocKind) | (ValKind << BaseBits)) {}
+  explicit SVal(SValKind Kind, const void *Data = nullptr)
+      : Data(Data), Kind(Kind) {}
 
-  explicit SVal(BaseKind k, const void *D = nullptr) : Data(D), Kind(k) {}
+  template <typename T> const T *castDataAs() const {
+    return static_cast<const T *>(Data);
+  }
 
 public:
   explicit SVal() = default;
@@ -105,38 +86,25 @@ class SVal {
     return llvm::dyn_cast<T>(*this);
   }
 
-  unsigned getRawKind() const { return Kind; }
-  BaseKind getBaseKind() const { return (BaseKind) (Kind & BaseMask); }
-  unsigned getSubKind() const { return Kind >> BaseBits; }
+  SValKind getKind() const { return Kind; }
 
   // This method is required for using SVal in a FoldingSetNode.  It
   // extracts a unique signature for this SVal object.
   void Profile(llvm::FoldingSetNodeID &ID) const {
-    ID.AddInteger((unsigned) getRawKind());
+    ID.AddInteger((unsigned)getKind());
     ID.AddPointer(Data);
   }
 
-  bool operator==(SVal R) const {
-    return getRawKind() == R.getRawKind() && Data == R.Data;
-  }
-
+  bool operator==(SVal R) const { return Kind == R.Kind && Data == R.Data; }
   bool operator!=(SVal R) const { return !(*this == R); }
 
-  bool isUnknown() const {
-    return getRawKind() == UnknownValKind;
-  }
+  bool isUnknown() const { return getKind() == UnknownValKind; }
 
-  bool isUndef() const {
-    return getRawKind() == UndefinedValKind;
-  }
+  bool isUndef() const { return getKind() == UndefinedValKind; }
 
-  bool isUnknownOrUndef() const {
-    return getRawKind() <= UnknownValKind;
-  }
+  bool isUnknownOrUndef() const { return isUnknown() || isUndef(); }
 
-  bool isValid() const {
-    return getRawKind() > UnknownValKind;
-  }
+  bool isValid() const { return !isUnknownOrUndef(); }
 
   bool isConstant() const;
 
@@ -207,10 +175,24 @@ inline raw_ostream &operator<<(raw_ostream &os, clang::ento::SVal V) {
   return os;
 }
 
+namespace nonloc {
+/// Sub-kinds for NonLoc values.
+#define NONLOC_SVAL(Id, Parent)                                                \
+  inline constexpr auto Id##Kind = SVal::SValKind::NonLoc##Id##Kind;
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
+} // namespace nonloc
+
+namespace loc {
+/// Sub-kinds for Loc values.
+#define LOC_SVAL(Id, Parent)                                                   \
+  inline constexpr auto Id##Kind = SVal::SValKind::Loc##Id##Kind;
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.def"
+} // namespace loc
+
 class UndefinedVal : public SVal {
 public:
   UndefinedVal() : SVal(UndefinedValKind) {}
-  static bool classof(SVal V) { return V.getBaseKind() == UndefinedValKind; }
+  static bool classof(SVal V) { return V.getKind() == UndefinedValKind; }
 };
 
 class DefinedOrUnknownSVal : public SVal {
@@ -223,16 +205,15 @@ class DefinedOrUnknownSVal : public SVal {
   static bool classof(SVal V) { return !V.isUndef(); }
 
 protected:
-  explicit DefinedOrUnknownSVal(const void *d, bool isLoc, unsigned ValKind)
-      : SVal(d, isLoc, ValKind) {}
-  explicit DefinedOrUnknownSVal(BaseKind k, void *D = nullptr) : SVal(k, D) {}
+  explicit DefinedOrUnknownSVal(SValKind Kind, const void *Data = nullptr)
+      : SVal(Kind, Data) {}
 };
 
 class UnknownVal : public DefinedOrUnknownSVal {
 public:
   explicit UnknownVal() : DefinedOrUnknownSVal(UnknownValKind) {}
 
-  static bool classof(SVal V) { return V.getBaseKind() == UnknownValKind; }
+  static bool classof(SVal V) { return V.getKind() == UnknownValKind; }
 };
 
 class DefinedSVal : public DefinedOrUnknownSVal {
@@ -246,22 +227,21 @@ class DefinedSVal : public DefinedOrUnknownSVal {
   static bool classof(SVal V) { return !V.isUnknownOrUndef(); }
 
 protected:
-  explicit DefinedSVal(const void *d, bool isLoc, unsigned ValKind)
-      : DefinedOrUnknownSVal(d, isLoc, ValKind) {}
+  explicit DefinedSVal(SValKind Kind, const void *Data)
+      : DefinedOrUnknownSVal(Kind, Data) {}
 };
 
 /// Represents an SVal that is guaranteed to not be UnknownVal.
 class KnownSVal : public SVal {
 public:
-  KnownSVal(const DefinedSVal &V) : SVal(V) {}
-  KnownSVal(const UndefinedVal &V) : SVal(V) {}
+  /*implicit*/ KnownSVal(DefinedSVal V) : SVal(V) {}
+  /*implicit*/ KnownSVal(UndefinedVal V) : SVal(V) {}
   static bool classof(SVal V) { return !V.isUnknown(); }
 };
 
 class NonLoc : public DefinedSVal {
 protected:
-  explicit NonLoc(unsigned SubKind, const void *d)
-      : DefinedSVal(d, false, SubKind) {}
+  NonLoc(SValKind Kind, const void *Data) : DefinedSVal(Kind, Data) {}
 
 public:
   void dumpToStream(raw_ostream &Out) const;
@@ -271,13 +251,14 @@ class NonLoc : public DefinedSVal {
            T->isAnyComplexType() || T->isVectorType();
   }
 
-  static bool classof(SVal V) { return V.getBaseKind() == NonLocKind; }
+  static bool classof(SVal V) {
+    return BEGIN_NonLoc <= V.getKind() && V.getKind() <= END_NonLoc;
+  }
 };
 
 class Loc : public DefinedSVal {
 protected:
-  explicit Loc(unsigned SubKind, const void *D)
-      : DefinedSVal(const_cast<void *>(D), true, SubKind) {}
+  Loc(SValKind Kind, const void *Data) : DefinedSVal(Kind, Data) {}
 
 public:
   void dumpToStream(raw_ostream &Out) const;
@@ -287,7 +268,9 @@ class Loc : public DefinedSVal {
            T->isReferenceType() || T->isNullPtrType();
   }
 
-  static bool classof(SVal V) { return V.getBaseKind() == LocKind; }
+  static bool classof(SVal V) {
+    return BEGIN_Loc <= V.getKind() && V.getKind() <= END_Loc;
+  }
 };
 
 //==------------------------------------------------------------------------==//
@@ -300,9 +283,9 @@ namespace nonloc {
 class SymbolVal : public NonLoc {
 public:
   SymbolVal() = delete;
-  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) {
-    assert(sym);
-    assert(!Loc::isLocType(sym->getType()));
+  explicit SymbolVal(SymbolRef Sym) : NonLoc(SymbolValKind, Sym) {
+    assert(Sym);
+    assert(!Loc::isLocType(Sym->getType()));
   }
 
   LLVM_ATTRIBUTE_RETURNS_NONNULL
@@ -314,27 +297,17 @@ class SymbolVal : public NonLoc {
     return !isa<SymbolData>(getSymbol());
   }
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == NonLocKind && V.getSubKind() == SymbolValKind;
-  }
-
-  static bool classof(NonLoc V) { return V.getSubKind() == SymbolValKind; }
+  static bool classof(SVal V) { return V.getKind() == SymbolValKind; }
 };
 
 /// Value representing integer constant.
 class ConcreteInt : public NonLoc {
 public:
-  explicit ConcreteInt(const llvm::APSInt& V) : NonLoc(ConcreteIntKind, &V) {}
-
-  const llvm::APSInt& getValue() const {
-    return *static_cast<const llvm::APSInt *>(Data);
-  }
+  explicit ConcreteInt(const llvm::APSInt &V) : NonLoc(ConcreteIntKind, &V) {}
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == NonLocKind && V.getSubKind() == ConcreteIntKind;
-  }
+  const llvm::APSInt &getValue() const { return *castDataAs<llvm::APSInt>(); }
 
-  static bool classof(NonLoc V) { return V.getSubKind() == ConcreteIntKind; }
+  static bool classof(SVal V) { return V.getKind() == ConcreteIntKind; }
 };
 
 class LocAsInteger : public NonLoc {
@@ -344,29 +317,20 @@ class LocAsInteger : public NonLoc {
       : NonLoc(LocAsIntegerKind, &data) {
     // We do not need to represent loc::ConcreteInt as LocAsInteger,
     // as it'd collapse into a nonloc::ConcreteInt instead.
-    assert(data.first.getBaseKind() == LocKind &&
-           (data.first.getSubKind() == loc::MemRegionValKind ||
-            data.first.getSubKind() == loc::GotoLabelKind));
+    SValKind K = data.first.getKind();
+    assert(K == loc::MemRegionValKind || K == loc::GotoLabelKind);
   }
 
 public:
   Loc getLoc() const {
-    const std::pair<SVal, uintptr_t> *D =
-      static_cast<const std::pair<SVal, uintptr_t> *>(Data);
-    return D->first.castAs<Loc>();
+    return castDataAs<std::pair<SVal, uintptr_t>>()->first.castAs<Loc>();
   }
 
   unsigned getNumBits() const {
-    const std::pair<SVal, uintptr_t> *D =
-      static_cast<const std::pair<SVal, uintptr_t> *>(Data);
-    return D->second;
+    return castDataAs<std::pair<SVal, uintptr_t>>()->second;
   }
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == NonLocKind && V.getSubKind() == LocAsIntegerKind;
-  }
-
-  static bool classof(NonLoc V) { return V.getSubKind() == LocAsIntegerKind; }
+  static bool classof(SVal V) { return V.getKind() == LocAsIntegerKind; }
 };
 
 class CompoundVal : public NonLoc {
@@ -379,19 +343,14 @@ class CompoundVal : public NonLoc {
 public:
   LLVM_ATTRIBUTE_RETURNS_NONNULL
   const CompoundValData* getValue() const {
-    return static_cast<const CompoundValData *>(Data);
+    return castDataAs<CompoundValData>();
   }
 
   using iterator = llvm::ImmutableList<SVal>::iterator;
-
   iterator begin() const;
   iterator end() const;
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == NonLocKind && V.getSubKind() == CompoundValKind;
-  }
-
-  static bool classof(NonLoc V) { return V.getSubKind() == CompoundValKind; }
+  static bool classof(SVal V) { return V.getKind() == CompoundValKind; }
 };
 
 class LazyCompoundVal : public NonLoc {
@@ -405,7 +364,7 @@ class LazyCompoundVal : public NonLoc {
 public:
   LLVM_ATTRIBUTE_RETURNS_NONNULL
   const LazyCompoundValData *getCVData() const {
-    return static_cast<const LazyCompoundValData *>(Data);
+    return castDataAs<LazyCompoundValData>();
   }
 
   /// It might return null.
@@ -414,14 +373,7 @@ class LazyCompoundVal : public NonLoc {
   LLVM_ATTRIBUTE_RETURNS_NONNULL
   const TypedValueRegion *getRegion() const;
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == NonLocKind &&
-           V.getSubKind() == LazyCompoundValKind;
-  }
-
-  static bool classof(NonLoc V) {
-    return V.getSubKind() == LazyCompoundValKind;
-  }
+  static bool classof(SVal V) { return V.getKind() == LazyCompoundValKind; }
 };
 
 /// Value representing pointer-to-member.
@@ -459,14 +411,7 @@ class PointerToMember : public NonLoc {
   iterator begin() const;
   iterator end() const;
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == NonLocKind &&
-           V.getSubKind() == PointerToMemberKind;
-  }
-
-  static bool classof(NonLoc V) {
-    return V.getSubKind() == PointerToMemberKind;
-  }
+  static bool classof(SVal V) { return V.getKind() == PointerToMemberKind; }
 
 private:
   explicit PointerToMember(const PTMDataType D)
@@ -487,29 +432,23 @@ class GotoLabel : public Loc {
     assert(Label);
   }
 
-  const LabelDecl *getLabel() const {
-    return static_cast<const LabelDecl *>(Data);
-  }
+  const LabelDecl *getLabel() const { return castDataAs<LabelDecl>(); }
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == LocKind && V.getSubKind() == GotoLabelKind;
-  }
-
-  static bool classof(Loc V) { return V.getSubKind() == GotoLabelKind; }
+  static bool classof(SVal V) { return V.getKind() == GotoLabelKind; }
 };
 
 class MemRegionVal : public Loc {
 public:
-  explicit MemRegionVal(const MemRegion* r) : Loc(MemRegionValKind, r) {
+  explicit MemRegionVal(const MemRegion *r) : Loc(MemRegionValKind, r) {
     assert(r);
   }
 
   /// Get the underlining region.
-  const MemRegion *getRegion() const {
-    return static_cast<const MemRegion *>(Data);
-  }
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
+  const MemRegion *getRegion() const { return castDataAs<MemRegion>(); }
 
   /// Get the underlining region and strip casts.
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
   const MemRegion* stripCasts(bool StripBaseCasts = true) const;
 
   template <typename REGION>
@@ -525,26 +464,16 @@ class MemRegionVal : public Loc {
     return getRegion() != R.getRegion();
   }
 
-  static bool classof(SVal V) {
-    return V.getBaseKind() == LocKind && V.getSubKind() == MemRegionValKind;
-  }
-
-  static bool classof(Loc V) { return V.getSubKind() == MemRegionValKind; }
+  static bool classof(SVal V) { return V.getKind() == MemRegionValKind; }
 };
 
 class ConcreteInt : public Loc {
 public:
-  explicit ConcreteInt(const llvm::APSInt& V) : Loc(ConcreteIntKind, &V) {}
+  explicit ConcreteInt(const llvm::APSInt &V) : Loc(ConcreteIntKind, &V) {}
 
-  const llvm::APSInt &getValue() const {
-    return *static_cast<const llvm::APSInt *>(Data);
-  }
-
-  static bool classof(SVal V) {
-    return V.getBaseKind() == LocKind && V.getSubKind() == ConcreteIntKind;
-  }
+  const llvm::APSInt &getValue() const { return *castDataAs<llvm::APSInt>(); }
 
-  static bool classof(Loc V) { return V.getSubKind() == ConcreteIntKind; }
+  static bool classof(SVal V) { return V.getKind() == ConcreteIntKind; }
 };
 
 } // namespace loc
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
index d89d82626f72694..ba05af9157fbecc 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -114,7 +114,7 @@ nonloc::SymbolVal SValBuilder::makeNonLoc(const SymExpr *operand,
   assert(operand);
   assert(!Loc::isLocType(toTy));
   if (fromTy == toTy)
-    return operand;
+    return nonloc::SymbolVal(operand);
   return nonloc::SymbolVal(SymMgr.getCastSymbol(operand, fromTy, toTy));
 }
 
@@ -446,7 +446,7 @@ SVal SValBuilder::makeSymExprValNN(BinaryOperator::Opcode Op,
 }
 
 SVal SValBuilder::evalMinus(NonLoc X) {
-  switch (X.getSubKind()) {
+  switch (X.getKind()) {
   case nonloc::ConcreteIntKind:
     return makeIntVal(-X.castAs<nonloc::ConcreteInt>().getValue());
   case nonloc::SymbolValKind:
@@ -458,7 +458,7 @@ SVal SValBuilder::evalMinus(NonLoc X) {
 }
 
 SVal SValBuilder::evalComplement(NonLoc X) {
-  switch (X.getSubKind()) {
+  switch (X.getKind()) {
   case nonloc::ConcreteIntKind:
     return makeIntVal(~X.castAs<nonloc::ConcreteInt>().getValue());
   case nonloc::SymbolValKind:
@@ -658,7 +658,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
   }
   SVal VisitUndefinedVal(UndefinedVal V) { return V; }
   SVal VisitUnknownVal(UnknownVal V) { return V; }
-  SVal VisitLocConcreteInt(loc::ConcreteInt V) {
+  SVal VisitConcreteInt(loc::ConcreteInt V) {
     // Pointer to bool.
     if (CastTy->isBooleanType())
       return VB.makeTruthVal(V.getValue().getBoolValue(), CastTy);
@@ -680,7 +680,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     // Pointer to whatever else.
     return UnknownVal();
   }
-  SVal VisitLocGotoLabel(loc::GotoLabel V) {
+  SVal VisitGotoLabel(loc::GotoLabel V) {
     // Pointer to bool.
     if (CastTy->isBooleanType())
       // Labels are always true.
@@ -707,7 +707,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     // Pointer to whatever else.
     return UnknownVal();
   }
-  SVal VisitLocMemRegionVal(loc::MemRegionVal V) {
+  SVal VisitMemRegionVal(loc::MemRegionVal V) {
     // Pointer to bool.
     if (CastTy->isBooleanType()) {
       const MemRegion *R = V.getRegion();
@@ -852,11 +852,11 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     // necessary for bit-level reasoning as well.
     return UnknownVal();
   }
-  SVal VisitNonLocCompoundVal(nonloc::CompoundVal V) {
+  SVal VisitCompoundVal(nonloc::CompoundVal V) {
     // Compound to whatever.
     return UnknownVal();
   }
-  SVal VisitNonLocConcreteInt(nonloc::ConcreteInt V) {
+  SVal VisitConcreteInt(nonloc::ConcreteInt V) {
     auto CastedValue = [V, this]() {
       llvm::APSInt Value = V.getValue();
       VB.getBasicValueFactory().getAPSIntType(CastTy).apply(Value);
@@ -878,11 +878,11 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     // Pointer to whatever else.
     return UnknownVal();
   }
-  SVal VisitNonLocLazyCompoundVal(nonloc::LazyCompoundVal V) {
+  SVal VisitLazyCompoundVal(nonloc::LazyCompoundVal V) {
     // LazyCompound to whatever.
     return UnknownVal();
   }
-  SVal VisitNonLocLocAsInteger(nonloc::LocAsInteger V) {
+  SVal VisitLocAsInteger(nonloc::LocAsInteger V) {
     Loc L = V.getLoc();
 
     // Pointer as integer to bool.
@@ -904,7 +904,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     const MemRegion *R = L.getAsRegion();
     if (!IsUnknownOriginalType && R) {
       if (CastTy->isIntegralOrEnumerationType())
-        return VisitLocMemRegionVal(loc::MemRegionVal(R));
+        return VisitMemRegionVal(loc::MemRegionVal(R));
 
       if (Loc::isLocType(CastTy)) {
         assert(Loc::isLocType(OriginalTy) || OriginalTy->isFunctionType() ||
@@ -918,7 +918,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     } else {
       if (Loc::isLocType(CastTy)) {
         if (IsUnknownOriginalType)
-          return VisitLocMemRegionVal(loc::MemRegionVal(R));
+          return VisitMemRegionVal(loc::MemRegionVal(R));
         return L;
       }
 
@@ -943,7 +943,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     // Pointer as integer to whatever else.
     return UnknownVal();
   }
-  SVal VisitNonLocSymbolVal(nonloc::SymbolVal V) {
+  SVal VisitSymbolVal(nonloc::SymbolVal V) {
     SymbolRef SE = V.getSymbol();
 
     const bool IsUnknownOriginalType = OriginalTy.isNull();
@@ -983,7 +983,7 @@ class EvalCastVisitor : public SValVisitor<EvalCastVisitor, SVal> {
     // Symbol to pointer and whatever else.
     return UnknownVal();
   }
-  SVal VisitNonLocPointerToMember(nonloc::PointerToMember V) {
+  SVal VisitPointerToMember(nonloc::PointerToMember V) {
     // Member pointer to whatever.
     return V;
   }
diff --git a/clang/lib/StaticAnalyzer/Core/SVals.cpp b/clang/lib/StaticAnalyzer/Core/SVals.cpp
index 2a43a01ff886182..0e1351215bb4247 100644
--- a/clang/lib/StaticAnalyzer/Core/SVals.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SVals.cpp
@@ -136,10 +136,10 @@ class TypeRetrievingVisitor
 public:
   TypeRetrievingVisitor(const ASTContext &Context) : Context(Context) {}
 
-  QualType VisitLocMemRegionVal(loc::MemRegionVal MRV) {
+  QualType VisitMemRegionVal(loc::MemRegionVal MRV) {
     return Visit(MRV.getRegion());
   }
-  QualType VisitLocGotoLabel(loc::GotoLabel GL) {
+  QualType VisitGotoLabel(loc::GotoLabel GL) {
     return QualType{Context.VoidPtrTy};
   }
   template <class ConcreteInt> QualType VisitConcreteInt(ConcreteInt CI) {
@@ -148,13 +148,7 @@ class TypeRetrievingVisitor
       return Context.BoolTy;
     return Context.getIntTypeForBitwidth(Value.getBitWidth(), Value.isSigned());
   }
-  QualType VisitLocConcreteInt(loc::ConcreteInt CI) {
-    return VisitConcreteInt(CI);
-  }
-  QualType VisitNonLocConcreteInt(nonloc::ConcreteInt CI) {
-    return VisitConcreteInt(CI);
-  }
-  QualType VisitNonLocLocAsInteger(nonloc::LocAsInteger LI) {
+  QualType VisitLocAsInteger(nonloc::LocAsInteger LI) {
     QualType NestedType = Visit(LI.getLoc());
     if (NestedType.isNull())
       return NestedType;
@@ -162,13 +156,13 @@ class TypeRetrievingVisitor
     return Context.getIntTypeForBitwidth(LI.getNumBits(),
                                          NestedType->isSignedIntegerType());
   }
-  QualType VisitNonLocCompoundVal(nonloc::CompoundVal CV) {
+  QualType VisitCompoundVal(nonloc::CompoundVal CV) {
     return CV.getValue()->getType();
   }
-  QualType VisitNonLocLazyCompoundVal(nonloc::LazyCompoundVal LCV) {
+  QualType VisitLazyCompoundVal(nonloc::LazyCompoundVal LCV) {
     return LCV.getRegion()->getValueType();
   }
-  QualType VisitNonLocSymbolVal(nonloc::SymbolVal SV) {
+  QualType VisitSymbolVal(nonloc::SymbolVal SV) {
     return Visit(SV.getSymbol());
   }
   QualType VisitSymbolicRegion(const SymbolicRegion *SR) {
@@ -281,30 +275,33 @@ void SVal::printJson(raw_ostream &Out, bool AddQuotes) const {
 }
 
 void SVal::dumpToStream(raw_ostream &os) const {
-  switch (getBaseKind()) {
-    case UnknownValKind:
-      os << "Unknown";
-      break;
-    case NonLocKind:
-      castAs<NonLoc>().dumpToStream(os);
-      break;
-    case LocKind:
-      castAs<Loc>().dumpToStream(os);
-      break;
-    case UndefinedValKind:
-      os << "Undefined";
-      break;
+  if (isUndef()) {
+    os << "Undefined";
+    return;
+  }
+  if (isUnknown()) {
+    os << "Unknown";
+    return;
+  }
+  if (NonLoc::classof(*this)) {
+    castAs<NonLoc>().dumpToStream(os);
+    return;
+  }
+  if (Loc::classof(*this)) {
+    castAs<Loc>().dumpToStream(os);
+    return;
   }
+  llvm_unreachable("Unhandled SVal kind!");
 }
 
 void NonLoc::dumpToStream(raw_ostream &os) const {
-  switch (getSubKind()) {
-    case nonloc::ConcreteIntKind: {
-      const auto &Value = castAs<nonloc::ConcreteInt>().getValue();
-      os << Value << ' ' << (Value.isSigned() ? 'S' : 'U')
-         << Value.getBitWidth() << 'b';
-      break;
-    }
+  switch (getKind()) {
+  case nonloc::ConcreteIntKind: {
+    const auto &Value = castAs<nonloc::ConcreteInt>().getValue();
+    os << Value << ' ' << (Value.isSigned() ? 'S' : 'U') << Value.getBitWidth()
+       << 'b';
+    break;
+  }
     case nonloc::SymbolValKind:
       os << castAs<nonloc::SymbolVal>().getSymbol();
       break;
@@ -360,21 +357,21 @@ void NonLoc::dumpToStream(raw_ostream &os) const {
     default:
       assert(false && "Pretty-printed not implemented for this NonLoc.");
       break;
-  }
+    }
 }
 
 void Loc::dumpToStream(raw_ostream &os) const {
-  switch (getSubKind()) {
-    case loc::ConcreteIntKind:
-      os << castAs<loc::ConcreteInt>().getValue().getZExtValue() << " (Loc)";
-      break;
-    case loc::GotoLabelKind:
-      os << "&&" << castAs<loc::GotoLabel>().getLabel()->getName();
-      break;
-    case loc::MemRegionValKind:
-      os << '&' << castAs<loc::MemRegionVal>().getRegion()->getString();
-      break;
-    default:
-      llvm_unreachable("Pretty-printing not implemented for this Loc.");
+  switch (getKind()) {
+  case loc::ConcreteIntKind:
+    os << castAs<loc::ConcreteInt>().getValue().getZExtValue() << " (Loc)";
+    break;
+  case loc::GotoLabelKind:
+    os << "&&" << castAs<loc::GotoLabel>().getLabel()->getName();
+    break;
+  case loc::MemRegionValKind:
+    os << '&' << castAs<loc::MemRegionVal>().getRegion()->getString();
+    break;
+  default:
+    llvm_unreachable("Pretty-printing not implemented for this Loc.");
   }
 }
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
index 3286d7f468f0cdd..8ca2cdb9d3ab7a7 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -63,7 +63,7 @@ ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State,
     return assumeSymUnsupported(State, Sym, Assumption);
   }
 
-  switch (Cond.getSubKind()) {
+  switch (Cond.getKind()) {
   default:
     llvm_unreachable("'Assume' not implemented for this NonLoc");
 
@@ -107,7 +107,7 @@ ProgramStateRef SimpleConstraintManager::assumeInclusiveRangeInternal(
     return assumeSymInclusiveRange(State, Sym, From, To, InRange);
   }
 
-  switch (Value.getSubKind()) {
+  switch (Value.getKind()) {
   default:
     llvm_unreachable("'assumeInclusiveRange' is not implemented"
                      "for this NonLoc");
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index aa2c8bbd15d4206..0f2d6c8fc80bb02 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -445,11 +445,11 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
     }
 
   while (true) {
-    switch (lhs.getSubKind()) {
+    switch (lhs.getKind()) {
     default:
       return makeSymExprValNN(op, lhs, rhs, resultTy);
     case nonloc::PointerToMemberKind: {
-      assert(rhs.getSubKind() == nonloc::PointerToMemberKind &&
+      assert(rhs.getKind() == nonloc::PointerToMemberKind &&
              "Both SVals should have pointer-to-member-type");
       auto LPTM = lhs.castAs<nonloc::PointerToMember>(),
            RPTM = rhs.castAs<nonloc::PointerToMember>();
@@ -465,36 +465,36 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
     }
     case nonloc::LocAsIntegerKind: {
       Loc lhsL = lhs.castAs<nonloc::LocAsInteger>().getLoc();
-      switch (rhs.getSubKind()) {
-        case nonloc::LocAsIntegerKind:
-          // FIXME: at the moment the implementation
-          // of modeling "pointers as integers" is not complete.
-          if (!BinaryOperator::isComparisonOp(op))
-            return UnknownVal();
-          return evalBinOpLL(state, op, lhsL,
-                             rhs.castAs<nonloc::LocAsInteger>().getLoc(),
-                             resultTy);
-        case nonloc::ConcreteIntKind: {
-          // FIXME: at the moment the implementation
-          // of modeling "pointers as integers" is not complete.
-          if (!BinaryOperator::isComparisonOp(op))
-            return UnknownVal();
-          // Transform the integer into a location and compare.
-          // FIXME: This only makes sense for comparisons. If we want to, say,
-          // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
-          // then pack it back into a LocAsInteger.
-          llvm::APSInt i = rhs.castAs<nonloc::ConcreteInt>().getValue();
-          // If the region has a symbolic base, pay attention to the type; it
-          // might be coming from a non-default address space. For non-symbolic
-          // regions it doesn't matter that much because such comparisons would
-          // most likely evaluate to concrete false anyway. FIXME: We might
-          // still need to handle the non-comparison case.
-          if (SymbolRef lSym = lhs.getAsLocSymbol(true))
-            BasicVals.getAPSIntType(lSym->getType()).apply(i);
-          else
-            BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
-          return evalBinOpLL(state, op, lhsL, makeLoc(i), resultTy);
-        }
+      switch (rhs.getKind()) {
+      case nonloc::LocAsIntegerKind:
+        // FIXME: at the moment the implementation
+        // of modeling "pointers as integers" is not complete.
+        if (!BinaryOperator::isComparisonOp(op))
+          return UnknownVal();
+        return evalBinOpLL(state, op, lhsL,
+                           rhs.castAs<nonloc::LocAsInteger>().getLoc(),
+                           resultTy);
+      case nonloc::ConcreteIntKind: {
+        // FIXME: at the moment the implementation
+        // of modeling "pointers as integers" is not complete.
+        if (!BinaryOperator::isComparisonOp(op))
+          return UnknownVal();
+        // Transform the integer into a location and compare.
+        // FIXME: This only makes sense for comparisons. If we want to, say,
+        // add 1 to a LocAsInteger, we'd better unpack the Loc and add to it,
+        // then pack it back into a LocAsInteger.
+        llvm::APSInt i = rhs.castAs<nonloc::ConcreteInt>().getValue();
+        // If the region has a symbolic base, pay attention to the type; it
+        // might be coming from a non-default address space. For non-symbolic
+        // regions it doesn't matter that much because such comparisons would
+        // most likely evaluate to concrete false anyway. FIXME: We might
+        // still need to handle the non-comparison case.
+        if (SymbolRef lSym = lhs.getAsLocSymbol(true))
+          BasicVals.getAPSIntType(lSym->getType()).apply(i);
+        else
+          BasicVals.getAPSIntType(Context.VoidPtrTy).apply(i);
+        return evalBinOpLL(state, op, lhsL, makeLoc(i), resultTy);
+      }
         default:
           switch (op) {
             case BO_EQ:
@@ -505,7 +505,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
               // This case also handles pointer arithmetic.
               return makeSymExprValNN(op, InputLHS, InputRHS, resultTy);
           }
-      }
+        }
     }
     case nonloc::ConcreteIntKind: {
       llvm::APSInt LHSValue = lhs.castAs<nonloc::ConcreteInt>().getValue();
@@ -765,8 +765,7 @@ static void assertEqualBitWidths(ProgramStateRef State, Loc RhsLoc,
       RhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(RhsLoc.getType(Ctx));
   uint64_t LhsBitwidth =
       LhsLoc.getType(Ctx).isNull() ? 0 : Ctx.getTypeSize(LhsLoc.getType(Ctx));
-  if (RhsBitwidth && LhsBitwidth &&
-      (LhsLoc.getSubKind() == RhsLoc.getSubKind())) {
+  if (RhsBitwidth && LhsBitwidth && (LhsLoc.getKind() == RhsLoc.getKind())) {
     assert(RhsBitwidth == LhsBitwidth &&
            "RhsLoc and LhsLoc bitwidth must be same!");
   }
@@ -812,7 +811,7 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
     }
   }
 
-  switch (lhs.getSubKind()) {
+  switch (lhs.getKind()) {
   default:
     llvm_unreachable("Ordering not implemented for this Loc.");
 
@@ -1372,7 +1371,7 @@ SVal SimpleSValBuilder::simplifySValOnce(ProgramStateRef State, SVal V) {
 
     SVal VisitMemRegion(const MemRegion *R) { return loc::MemRegionVal(R); }
 
-    SVal VisitNonLocSymbolVal(nonloc::SymbolVal V) {
+    SVal VisitSymbolVal(nonloc::SymbolVal V) {
       // Simplification is much more costly than computing complexity.
       // For high complexity, it may be not worth it.
       return Visit(V.getSymbol());
diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp
index 7577b7682a958ca..67ca61bb56ba287 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -402,7 +402,7 @@ SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) {
   Loc BaseL = Base.castAs<Loc>();
   const SubRegion* BaseR = nullptr;
 
-  switch (BaseL.getSubKind()) {
+  switch (BaseL.getKind()) {
   case loc::MemRegionValKind:
     BaseR = cast<SubRegion>(BaseL.castAs<loc::MemRegionVal>().getRegion());
     break;

>From d1d5c31b650fe5eac45f52311ed79fac0476273e Mon Sep 17 00:00:00 2001
From: Balazs Benics <benicsbalazs at gmail.com>
Date: Fri, 3 Nov 2023 12:15:43 +0100
Subject: [PATCH 2/2] Use llvm::to_underlying

---
 clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index 78239d4060c47d0..767843a18471e25 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -22,6 +22,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Casting.h"
 #include <cassert>
@@ -91,8 +92,8 @@ class SVal {
   // This method is required for using SVal in a FoldingSetNode.  It
   // extracts a unique signature for this SVal object.
   void Profile(llvm::FoldingSetNodeID &ID) const {
-    ID.AddInteger((unsigned)getKind());
     ID.AddPointer(Data);
+    ID.AddInteger(llvm::to_underlying(getKind()));
   }
 
   bool operator==(SVal R) const { return Kind == R.Kind && Data == R.Data; }



More information about the cfe-commits mailing list