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

Denys Petrov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 15 03:10:11 PDT 2020


ASDenysPetrov added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:734
   //        expressions which we currently do not know how to negate.
-  const RangeSet *getRangeForMinusSymbol(ProgramStateRef State, SymbolRef Sym) {
+  Optional<RangeSet> getRangeForInvertedSub(SymbolRef Sym) {
     if (const SymSymExpr *SSE = dyn_cast<SymSymExpr>(Sym)) {
----------------
NoQ wrote:
> vsavchenko wrote:
> > ASDenysPetrov wrote:
> > > vsavchenko wrote:
> > > > ASDenysPetrov wrote:
> > > > > As for me, I'd call this like `getRangeForNegatedSymSymExpr`, since you do Negate operation inside.
> > > > I'm not super happy about my name either, but I feel like it describes it better than the previous name and your version.  That function doesn't work for any `SymSymExpr` and it doesn't simply negate whatever we gave it.  It works specifically for symbolic subtractions and this is the information I want to be reflected in the name.
> > > Oh, I just assumed //...Sub// at the end as a //subexpression// but you mean //subtraction//. What I'm trying to say is that we can rename it like `getRangeFor...`//the expression which this function can handle//. E.g. `getRangeForNegatedSubtractionSymSymExpr`. My point is in a speaking name.
> > > 
> > > I think //invertion// is not something appropriate in terms of applying minus operator. I think invertion of zero should be something opposite but not a zero. Because when you would like to implement the function which turns [A, B] into [MIN, A)U(B, MAX], what would be the name of it? I think this is an //invertion//.
> > > 
> > > But this is not a big deal, it's just my thoughts.
> > My thought process here was that we are trying to get range for `A - B` and there is also information on `B - A`, so we can get something for `A - B` based on that.  So, it doesn't really matter what it does under the hood with ranges, it matters what its semantics are.  Here I called `B - A` //an inverted subtraction//.
> > I don't really know what would be the best name, but I thought that this one makes more sense.
> > Because when you would like to implement the function which turns [A, B] into [MIN, A)U(B, MAX], what would be the name of it? I think this is an //invertion//.
> 
> https://en.wikipedia.org/wiki/Complement_(set_theory)
> https://en.wikipedia.org/wiki/Inverse_function
> 
> "Negated subtraction" is definitely my favorite so far. "Mirrored" might be a good layman term as well.
>https://en.wikipedia.org/wiki/Complement_(set_theory)
>https://en.wikipedia.org/wiki/Inverse_function
Thanks for the links, @NoQ. That's much definite now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82381





More information about the cfe-commits mailing list