[cfe-commits] Fix PR13434 - mangling of templates with function pointers as arguments

[Timur Iskhodzhanov]

Attached is the current version of my patch.

On Thu, Jul 26, 2012 at 5:59 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> I've started with a small r160783 that ...
>
> On Mon, Jul 23, 2012 at 9:37 PM, John McCall <rjmccall at apple.com> wrote:
>> It would be good to at least come up with a reasonable-seeming
>> mangling for them.  Basically, please make sure they're mangled
>> just like pointers to functions, but different in some obvious way.
> ... adds an zeta/eta comparison between function pointer and block mangling ...
My current problem is that I don't quite understand what a block is.
Given the current structure of MicrosoftMangle and ItaniumMangle, the
following mangling makes some sense:

void zeta(int (*)(int, int)) {}
// CHECK: @"\01?zeta@@YAXP6AHHH at Z@Z"
void eta(int (^)(int, int)) {}
// CHECK: @"\01?eta@@YAXP_E$$A6AHHH at Z@Z"
// WAS:    @"\01?eta@@YAXP_EAHHH at Z@Z"

the interesting part goes after "YAX".
"P" currently means "pointer or member pointer or block pointer"

zeta:
"6" means "function pointer"
After appending "6" we call ``mangleType(FT, NULL, false, false);``
instead of ``mangle(FunctionProto, ...)``
- why?

eta:
"_E" means "block pointer"
After appending "_E" we call ``mangle(FunctionProto, ...)``
- why?

Is it true that BlockPointerType::getPointeeType is always a FunctionProto?
if yes - we can just call ``mangleType(FT, NULL, false, false);``.
if no - the current mangling makes perfect sense to me (is it possible
to have a block referencing a function pointer?)

Waiting for your suggestion or a green light to commit the current
patch with the "FIXME/FYI" comments.

> On Tue, Jul 24, 2012 at 12:53 AM, Charles Davis <cdavis at mymail.mines.edu> wrote:
>>
>> On Jul 23, 2012, at 5:52 AM, Timur Iskhodzhanov wrote:
>>
>>> Hi Charles, John,
>>>
>>> Can you please see
>>> http://llvm.org/bugs/show_bug.cgi?id=13434
>>> ?
>>> It seems like this case has never been tested before.
>>>
>>> I've written some tests and made a simple fix to the code.
>>>
>>> It's not thoroughly tested on all cases but IMO adding at least some
>>> new coverage and making all new tests pass by a simple is a good
>>> progress to start with.
>>>
>>> Two questions:
>>> 1) See the FIXME comment in MicrosoftMangle - I do think the whole
>>> comment should be made less cryptic.
>>>  Probably I'll need some help from Charles to decipher this :)
>> I already acted on that suggestion. You can safely remove the 'I'll probably have…' part.
> ... and removes the "I'll probably have" part.
>
>>> 2) See the zeta mangling in mangle-ms.cpp - not sure if changing the
>>> mangling there is a problem at all, as "blocks" are never
>>> compiled/mangled by MSVS (they are clang-specific extensions).
>> OK, but what about lambdas?
> The lambdas are just not supported in MicrosoftMangle (compared to
> ItaniumMangle) so I don't think mangling of templates with lambda
> parameters is a priority now? :)
>
>> Oh, and, as John said, the mangling of a block pointer should resemble that of a function pointer (i.e. no '$$A6').
> Yeah, I think so too now. Trying to achieve that...
>
>>> Can you please review the patch?
>> Seems to me that the '$$A' part is specific to template parameters that are neither pointers nor references to functions. What happens in those cases? The '6' is probably because this is a global (near) function. What happens when the NTTP is a member function? (Can you even do that?)
> Oh my god!
>
> ----------------------------------
> template<typename T> class Z {};
>
> class C {
>  public:
>   void method() {}
> };
>
> template<typename T> class Z<void (T::*)(void)> {};
> void foo(Z<void (C::*)(void)>) {}
> // CHECK: "\01?foo@@YAXV?$Z at P8C@@AEXXZ@@@Z"
> ----------------------------------
I've added a similar test case.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug_13434_2.patch
Type: application/octet-stream
Size: 3835 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120726/33b2abd3/attachment.obj>