r189208 - [-cxx-abi microsoft] Unnamed types are mangled less wrong

[David Majnemer]

On Wed, Aug 28, 2013 at 8:04 AM, David Blaikie <dblaikie at gmail.com> wrote:

> On Wed, Aug 28, 2013 at 2:06 AM, David Majnemer
> <david.majnemer at gmail.com> wrote:
> > On Mon, Aug 26, 2013 at 1:09 PM, Reid Kleckner <rnk at google.com> wrote:
> >>
> >> On Sun, Aug 25, 2013 at 8:03 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>>
> >>> On Sun, Aug 25, 2013 at 8:02 PM, David Majnemer
> >>> <david.majnemer at gmail.com> wrote:
> >>> > On Sun, Aug 25, 2013 at 7:59 PM, David Blaikie <dblaikie at gmail.com>
> >>> > wrote:
> >>> >>
> >>> >> On Sun, Aug 25, 2013 at 7:35 PM, David Majnemer
> >>> >> <david.majnemer at gmail.com> wrote:
> >>> >> > Author: majnemer
> >>> >> > Date: Sun Aug 25 21:35:51 2013
> >>> >> > New Revision: 189208
> >>> >> >
> >>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=189208&view=rev
> >>> >> > Log:
> >>> >> > [-cxx-abi microsoft] Unnamed types are mangled less wrong
> >>> >>
> >>> >> Test case?
> >>> >
> >>> >
> >>> > <unnamed-tag> is still wrong, <unnamed-tag>@ is just marginally less
> >>> > wrong.
> >>> > I thought of this change more of a code cleanup than a bug-fix for
> >>> > mangling.
> >>>
> >>> A change in behavior really ought to have a test. If it's still wrong,
> >>> a FIXME showing the ring mangling in the test case should suffice.
> >>>
> >>> The fact that this didn't break any existing tests seems to indicate
> >>> that this area lacks coverage - adding tests now, even if they
> >>> demonstrate the broken behavior & document what it should be, might be
> >>> nice, so we can track progress towards correctness.
> >>
> >>
> >> Right, I'd CHECK for the current mangling and have a FIXME with the
> >> desired mangling.
> >
> >
> > We already have a PR tracking the broken behavior. I am not aware of any
> > LLVM policy, codified or implicit, that asks for bug PRs to be encoded in
> > the test suite.
>
> There isn't, though it's been discussed as a possible idea (one that I
> rather like).
>
> The relevant policy here is: intentional behavioural changes have
> tests for the changed behaviour.
>

And I fully agree with that policy, I just didn't see it as a behavioral
change.  In my eyes, that code was still there purely for illustrative
reasons so that I remember to '@' terminate when I came back to fix it for
real.


> >
> > This particular mangling bug is fixed in
> > http://llvm-reviews.chandlerc.com/D1540
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130828/a34ef7c3/attachment.html>