[PATCH] SemaCXX: Support templates in availability attributes

[Aaron Ballman via cfe-commits]

On Tue, Feb 23, 2016 at 6:39 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>
>> On 2016-Feb-23, at 11:18, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Tue, Feb 23, 2016 at 1:52 PM, Manman Ren <mren at apple.com> wrote:
>>> This patch looks good to me. But I am not sure if Aaron has any comment.
>>>
>>> On Feb 22, 2016, at 6:19 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com>
>>> wrote:
>>>
>>>
>>> On 2016-Feb-22, at 17:24, Manman Ren <mren at apple.com> wrote:
>>>
>>>
>>>
>>> On Feb 8, 2016, at 8:17 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com>
>>> wrote:
>>>
>>> This patch adds support for templates in availability attributes.
>>> - If the context for an availability diagnostic is a
>>> `FunctionTemplateDecl`, look through it to the `FunctionDecl`.
>>>
>>>
>>> AvailabilityResult Decl::getAvailability(std::string *Message) const {
>>> +  if (auto *FTD = dyn_cast<FunctionTemplateDecl>(this))
>>> +    return FTD->getTemplatedDecl()->getAvailability(Message);
>>> +
>>> AvailabilityResult Result = AR_Available;
>>>
>>>
>>> This looks generally correct to me.
>>> The UnavailableAttr is attached to the FunctionDecl, not the
>>> FunctionTemplateDecl, so looking through sounds right.
>>>
>>> - Add `__has_feature(attribute_availability_in_templates)`.
>>>
>>>
>>> @Aaron, any comment on this?
>>> This patch adds extra support for Availability attribute (similar to
>>> attribute_availability_with_strict in r261548).
>>> Not sure if has_attribute can be used for this purpose.
>>
>> Given that we're already using __has_feature for the rest of the
>> availability attribute stuff, I think it's better to keep it all
>> grouped together instead of checking for some features with
>> __has_feature and others with __has_attribute.
>
> Besides that argument, this isn't adding any attributes, just checking
> how they behave.
>
>> If I understand
>> properly, this is taking code that would have previously been
>> ill-formed and making it well-formed, and that's why the feature
>> testing macro is required?
>
> Exactly.  Previously this was ill-formed:
> ```
> class Unavail __attribute__((unavailable));
>
> template <class T>
> void foo(Unavail&) __attribute__((unavailable));
> ```
>
> Same for `__attribute((availability(macosx,unavailable)))`, and other
> triggers of "unavailable".

Okay, this makes sense to me, thank you for the explanation. LGTM!

~Aaron
>
>>
>> Thanks!
>>
>> ~Aaron
>>
>>>
>>> Manman
>>>
>>>
>>> Is there anything else I should be testing to be sure availability
>>> works correctly in templates?
>>>
>>>
>>> Maybe
>>> test<UnavailableClass>()
>>> calling unavailable function from an unavailable template function
>>> calling an unavailable template function
>>>
>>> I think these all work with the current compiler. But I am not sure if we
>>> have existing test coverage.
>>>
>>>
>>> Thanks for the ideas; let me know if you have any others.
>>>
>>> Can you have a look at the new patch?
>>>
>>> Cheers,
>>> Manman
>>>
>>> I'm working on a patch to add
>>> availability markup to the libc++ headers, and this is the only
>>> problem I've hit so far.  Anyone have thoughts on other things I
>>> should test?
>>>
>>>
>>> <0001-SemaCXX-Support-templates-in-availability-attributes.patch>
>