[PATCH] D92606: Clean up debug locations for logical-and/or expressions

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.


================
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 have no insight here.  I'm willing to take it on as a follow-up exercise, after the FastISel patch goes back in.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92606/new/

https://reviews.llvm.org/D92606