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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
