[PATCH] D103094: [analyzer] Implemented RangeSet::Factory::castTo function to perform promotions, truncations and conversions.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:692
+  ContainerType Result;
+  ContainerType RHS;
+
----------------
ASDenysPetrov wrote:
> vsavchenko wrote:
> > We have LHS (left-hand side) and RHS (right-hand side) as a common convention for naming operands of binary operators.
> > The fact that you later on use it as an RHS for `unite`, doesn't make it a good name.
> What about `Tmp`?
Maybe something like `Dummy` or `Intermediate`.


================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:129-141
+  template <typename T> RangeSet from(APSIntType Ty, RawRangeSetT<T> Init) {
+    llvm::APSInt First, Second;
+    Ty.apply(First);
+    Ty.apply(Second);
     RangeSet RangeSet = F.getEmptySet();
     for (const auto &Raw : Init) {
+      First = Raw.first;
----------------
ASDenysPetrov wrote:
> vsavchenko wrote:
> > I don't really understand what's going on here.
> This is a generalized version of `from` function. But this one you can use independently from which type the current test case uses. You can set a custom type to get a range set for a given type. This allows to get range sets based on different types (e.g. `char` and `int`)  in a single test case.
> Previously `from` produced range sets only for the type specified by a test case.
OK then, if you make this `from` templated, make all of them templated and you won't need to duplicate the logic with `sizeof * 8` everywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103094