dblaikie added a comment.
Probably worth separating these changes/fixes/tests (looks like 3 different
changes?) - at least it'd help me understand what each one of the changes does,
separately (I'm especially curious about the EmitScalarConversion one and would
like to better understand why the generic Expr handling for ApplyDebugLocation
isn't working here, if it can be made more robust and/or if the proposed
solution could be generalized a bit/moved further up in the stack). And is
there an existing test file/cases these could be put near?
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4293
+ {
+ // There is no need to emit line number for unconditional branch.
+ auto NL = ApplyDebugLocation::CreateEmpty(CGF);
----------------
rnk wrote:
> I see this is consistent with LAnd handling above, but is there a principled
> reason for using an empty location here? If I was stopped on this
> unconditional branch instruction in the debugger, I would expect to see the
> location of the logical operator.
>
> No need to change behavior in this patch, I'm just wondering if we can remove
> this as a followup.
>
> I dug into the blame, and the comment about "no need" for a location comes
> from 2011:
> http://github.com/llvm/llvm-project/commit/4d7612744f080d2c0a8639c1f5e41b691e1bba55
> The reasoning seems to be that it improves the gdb test suite.
I /think/ I remember encountering these issues independently/as well when I was
working on the gdb test suite maybe a couple of years later. Hmm, guess it
might've overlapped with this work.
6f2e41e0d4421ab376801bfb88d3f197a8e96994 / r128471 provides maybe a bit more
context: "Do not line number entry for unconditional branches. Usually, users
do not want to stop at closing '}'."
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92606/new/
https://reviews.llvm.org/D92606
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
