On 10/11/2017 05:14 AM, Nathan Sidwell wrote:
> On 10/04/2017 03:40 PM, Martin Sebor wrote:
>> On 09/28/2017 08:25 AM, Nathan Sidwell wrote:
>
>
>> Since the original tests (where the resolver returns void*) pass
>> across the board I assume it must work for all supported ABIs.
>> Or is there some subtlety between the before and after code that
>> I'm missing?
>
> I had a vague notion of some (IBM?) target that might do something
> different. Perhaps it's gone. Your comment explains it fine.
>
>>> oh, I think it's trying to spot the pointer to NON-static member
>>> function internal record type. But brokenly. I think pmf record_types
>>> have DECL_ARTIFICIAL and BUILTIN_LOCATION, that might be useful.
>>
>> It turns out this code was superfluous with the C++ correction
>> and I was able to remove it with no impact on the tests.
>
> Great.
>
>>> Is this function called before we know whether we've enabled the
>>> appropriate warnings? It would be better to bail out sooner if the
>>> warnings are disabled.
>>
>> I'm not sure I understand the question or suggestion here but
>> warnings in general are certainly enabled at this point. The
>> function issues both errors and warnings so it can't very well
>> exit early without checking the compatibility. Let me know if
>> I misunderstood your comment.
>
> Oh, forgot it issued errors too, so my queston is moot.
>
>> -signedness.
>> +signedness. In C++ where incompatible pointer conversions ordinarily
>> cause
> Missing comma: In C++, where incompatible ...
>
>> +errors, the warning detects such conversions in GCC extensions that
>> aren't
>> +part of the standard language. An example of a construct that might
>> perform
>> +such a conversion is the @code{alias} attribute. @xref{Function
>> Attributes,,Declaring Attributes of Functions}.
>
> Looks fine to me. (pedantically, I don't think I can formally approve
> this, because it's not part of the C++ FE.)
That's good enough for me :-) So I'll make it an official ACK.
jeff
