[cfe-commits] [Windows] Fix PR13389 - wrong mangling of return type qualifiers

Timur Iskhodzhanov timurrrr at google.com
Thu Jul 19 13:39:14 PDT 2012


On Thu, Jul 19, 2012 at 11:22 PM, John McCall <rjmccall at apple.com> wrote:
> On Jul 19, 2012, at 4:44 AM, Timur Iskhodzhanov wrote:
>> On Thu, Jul 19, 2012 at 3:37 PM, João Matos <ripzonetriton at gmail.com> wrote:
>>> Not sure who is responsible for the MS mangler, the LLVM code owner
>>> list doesn't list anyone directly responsible for non-codegen Windows
>>> stuff. It would be nice to see someone assigned to this task, at the
>>> moment the Windows patches take a lot of time to be reviewed or get
>>> forgotten on the list.
>> I totally agree here.
>
> It's me, and I'm afraid you'll just have to deal with me not being very
> responsive.  I have a lot of things to watch and not as much time to do so
> as I might like.
ok

João,
Please prefer "I like the patch but wait for John's review" over
"Looks good to me" then :)

>>> By the way, I don't think we need to enable blocks support in this test case.
>> Good point!
>> I've removed the unnecessary flags from the cc1 invocation, see the
>> updated patch
>
> I don't really understand what's going on in this patch at all.  If there's a
> consistent rule here, it'd be nice to see a comment talking about it.
Ooops - I wanted to update the comment but forgotten to.
Will do tomorrow.

> Otherwise it seems very, very ad-hoc,
I do think this is more of a general rule as the same ABCD encoding
should be used to fix http://llvm.org/PR13182 too.
Maybe I should merge these two fixes into one patch and extract a
"mangleCVQualifiers" function? WDYT?
I was planning to have this as two separate patches.

> which makes me worried that
> this it only works for the test cases we've run through it already.
That's TDD, right? :)
I build Chromium tests to generate test cases - I get lots of
link-time errors when the mangler is wrong.
The suggested patch does fix mangling of return types needed to have a
mixed cl/clang build of Chromium and I haven't observed any other/new
return type mangling problems since applying the fix locally.

My rationale of the TDD/a-bit-ad-hoc approach is that we constantly
and incrementally move forward and don't regress on known manglings.
I don't think it's immediately possible to come up with "one patch to
fix them all" :)

Suggestions are welcome!

> John.




More information about the cfe-commits mailing list