[PATCH] D69540: [analyzer] DynamicSize: Remove 'getExtent()' from regions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:158
               // symbols to use, only content metadata.
-              return nonloc::SymbolVal(SymMgr.getExtentSymbol(FTR));
+              return FTR->getMemRegionManager().getStaticSize(FTR);
 
----------------
Charusso wrote:
> NoQ wrote:
> > Charusso wrote:
> > > That is the breaking test's code, which is super wonky. I cannot understand what is the rational behind this concept.
> > Your new code would return a concrete integer here:
> > ```lang=c++
> >   case MemRegion::FunctionCodeRegionKind: {
> >     QualType Ty = cast<TypedRegion>(SR)->getDesugaredLocationType(Ctx);
> >     return getTypeSize(Ty, Ctx, SVB);
> >   }
> > ```
> > Previously it was a symbol.
> > 
> > That said, the original code looks like a super gross hack: they used an extent symbol not because they actually needed an extent, but because they didn't have a better symbol to use :/ I guess you should just keep the extent symbol for now :/
> I see, that was really missing, whoops. Thanks!
Let's keep the hack here, not move it to the region manager. I.e., please create `SymbolExtent` here directly and then separately keep creating it in `getStaticSize()`. I.e., don't reuse the code when it isn't supposed to behave identically, even if right now it accidentally does behave identically.


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

https://reviews.llvm.org/D69540