dcoughlin added inline comments.
================
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:694
+ } else if (Optional<BlockEntrance> BE = P.getAs<BlockEntrance>()) {
+ CFGElement BlockFront = BE->getBlock()->front();
+ if (BlockFront.getKind() == CFGElement::Statement) {
----------------
MTC wrote:
> dcoughlin wrote:
> > MTC wrote:
> > > szepet wrote:
> > > > I think it would be more correct to use the location what is used in
> > > > case of the BlockEdge. (So on the entranced block terminator
> > > > condition.) The reason is because the BlockEntrance display message
> > > > will be displayed before the message of the BlockEdge (since it is an
> > > > "earlier" node in the ExplodedGraph). So it would result that if check
> > > > these notes in a viewer then the earlier note would belong to the later
> > > > location which could be confusing.
> > > Yes, it would be better to use the location of the TerminatorCondition :D.
> > Thanks for looking into fixing this.
> >
> > I don't think using the terminator condition of the entered block is the
> > right thing to do. The terminator is at the *end* of a basic block and this
> > program point represents the entrance to the block. I think it is better to
> > use the location corresponding to the first element in in the entered block.
> Thank you, dcoughlin!
>
> I have updated the diff. The first element kind I can think of is only
> `Stmt` and `NewAllocator`. I don't know if it's enough?
>
It would be good to add an llvm_unreachable() assertion expressing this
expectation so that we'll get an assertion failure if this turns out not to be
enough. (Or if we add a new CFGElement kind in the future).
================
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:695
+ CFGElement BlockFront = BE->getBlock()->front();
+ if (BlockFront.getKind() == CFGElement::Kind::Statement) {
+ return PathDiagnosticLocation(
----------------
You can use `getAs<>` in the `if` guard condition to make this more concise:
```
if (auto StmtElt = BlockFront.getAs<CFGStmt>()) {
return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
}
```
https://reviews.llvm.org/D37187
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
