NoQ added a comment.
> Do you think its ever okay to find a last store in a `BlockEdge`?
*In a `BlockEntrance` into an empty block (that has no elements but does have a
terminator). At least that's what your code is fixing.
Attaching store notes to such program points is most likely not ok. We can
always make ourselves a `WideningPoint` for that specific purpose, or maybe
even `PreWidening` / `PostWidening`. Program points are not set in stone, we
are free to make as many kinds of them as we need (which is why we also have
tags).
Being able to attach an event note at all to such program point is most likely
ok. Any program point can potentially represent an interesting event. So this
is anyway a good change. I'd love to have some more careful testing, maybe a
unittest (or somehow trick -verify with line breaks), so that to know where
exactly does the note go in this case (is it the jump from the bottom of the
loop? is it jump from increment to condition?). This way we'll make sure that
the implementation is correct.
I'd still want you to figure out why is this widening-specific. Do i understand
correctly that we're doing widening in an inappropriate moment of time? I'd
prefer to have this confirmed. Or can we reproduce this crash / incorrect
behavior / false positive without widening?
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1805
// the only thing that leads to is the addition of calls to operator!=.
- if (isa<CXXForRangeStmt>(NB->getTerminator()))
return nullptr;
----------------
Why did this even compile? I thought i deleted these conversions in D61814 >.<
================
Comment at: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp:774-775
+
+ return PathDiagnosticLocation(
+ BE->getBlock()->getTerminatorStmt()->getBeginLoc(), SMng);
} else if (Optional<FunctionExitPoint> FE = P.getAs<FunctionExitPoint>()) {
----------------
What exactly is the terminator here in your case? Is it `NullStmt`? Is there
always a terminator and/or a terminator statement?
What if that's a `BlockEntrance` to the exit-block? (do we even make such
program points? i hope we don't)
I think this code needs comments in order to explain what picture do we have in
mind.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66716/new/
https://reviews.llvm.org/D66716
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
