Fwd: [PATCH] Proposed fix for PR23076 (conditional branch debug line info)

[David Blaikie via cfe-commits]

Forward to the 'new' mailing list.
---------- Forwarded message ----------
From: David Blaikie <dblaikie at gmail.com>
Date: Fri, Jan 15, 2016 at 6:08 PM
Subject: Re: [PATCH] Proposed fix for PR23076 (conditional branch debug
line info)
To: reviews+D8822+public+818b95c17195c725 at reviews.llvm.org, "Robinson,
Paul" <Paul_Robinson at playstation.sony.com>, Adrian Prantl <aprantl at apple.com
>
Cc: "Pieb, Wolfgang" <wolfgang_pieb at playstation.sony.com>, llvm cfe <
cfe-commits at cs.uiuc.edu>


Apologies for the massively delayed review. I think this is probably good,
but here are some test cases to discuss (& I've cc'd Paul Robinson and
Adrian Prantl, both who work on debug info in LLVM as well, to get their
thoughts)

Given this code:

 1. int main() {
 2.   if (
 3.       tr()
 4.       &&
 5.       tr()
 6.       )
 7.     func();
 8.
 9.   if (
10.       fa()
11.       &&
12.       tr()
13.       )
14.     func();
15. }

G++-4.8:
3, 4, 5, 4, 2, 7
10, 11, 9, 15

Clang before the change:
3, 4, 5, 3, 7
10, 11, 15

Clang after the change:
3, 5, 7
10, 15

The lack of stepping between the evaluation of the LHS and the RHS to the
&& expression seems like a loss, but not a great one. How's everyone else
feel about it? (I imagine we can, with some work, preserve that step while
avoiding the weird backwards step to the start of the expression before
continuing)

On Fri, May 1, 2015 at 1:00 PM, Wolfgang Pieb <
wolfgang_pieb at playstation.sony.com> wrote:

> Hi David, did you have a chance to look at the new patch? There are a
> couple of new tests piggy-backing on the existing test case.
>
>
> http://reviews.llvm.org/D8822
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160115/c23bf553/attachment-0001.html>