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

David Blaikie dblaikie at gmail.com
Wed Aug 28 08:04:35 PDT 2013


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.

>
> This particular mangling bug is fixed in
> http://llvm-reviews.chandlerc.com/D1540



More information about the cfe-commits mailing list