Szelethus added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:552 + + Index = StackFrame->getIndex(); + ---------------- baloghadamsoftware wrote: > Szelethus wrote: > > This mustn't be serious. `StackFrameContext::getIndex()` has **no > > comments** at all! So it is an index to the element within a `CFGBlock` to > > which it also stores a field for??? Ugh. Shouldn't we just have a > > `getCallSiteCFGElement` or something? Especially since we have > > `CFGElementRef`s now. > > > > Would you be so nice to hunt this down please? :) > Do you mean a new `StackFrameContext::getCFGElement()` member function? I > agree. I can do it, however this technology where we get the block and the > index separately is used at many places in the code, then it would be > appropriate to refactor all these places. Even wrose is the backward > direction, where we look for the CFG element of a statement: we find the > block from `CFGStmtMap` and then we search for the index **liearly**, instrad > of storing it in the map as well!!! Nasty. Yeah, I mean not to burden you work refactoring any more then you feel like doing it, but simply adding a `StackFrameContext::getCFGElement()` method and using it in this patch would be nice enough, and some comments to `getIndex()`. But this obviously isn't a high prio issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80366/new/ https://reviews.llvm.org/D80366 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
