[PATCH] D82381: [analyzer] Introduce small improvements to the solver infra

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:373
+  ++Upper;
+  --Lower;
+
----------------
Sorry if my question is dumb, but I do not really have a mental model at this point about where do we actually handle types and overflows. Will this method work when we delete the last or the first element of the full range of a type?

I think having unit tests would be a great way to make this clear. I always felt that the solver is actually something that should be really easy to test separately and those tests would also help a lot to understand how the solver is actually working. 


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:467-470
+/// Available representations for the arguments are:
+///   * RangeSet
+///   * Optional<RangeSet>
+///   * RangeSet *
----------------
NoQ wrote:
> Why do we have these representations in the first place?
> 
> It sounds like you're treating null pointers / empty optionals equivalently to full ranges (i.e., intersecting with them has no effect). How hard would it be to simply construct an actual full range in all the places in which we currently have empty optionals?
+1

I also find this confusing. Should I interpret a None as a full or empty range? Does this depend on the context or a general rule? Is there any reason we need to handle nullable pointers or could we actually make call sites more uniform to get rid of some of the complexity here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82381