> On 2016-Feb-24, at 10:01, Aaron Ballman <aa...@aaronballman.com> wrote: > > On Tue, Feb 23, 2016 at 6:39 PM, Duncan P. N. Exon Smith > <dexonsm...@apple.com> wrote: >> >>> On 2016-Feb-23, at 11:18, Aaron Ballman <aa...@aaronballman.com> wrote: >>> >>> On Tue, Feb 23, 2016 at 1:52 PM, Manman Ren <m...@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 <dexonsm...@apple.com> >>>> wrote: >>>> >>>> >>>> On 2016-Feb-22, at 17:24, Manman Ren <m...@apple.com> wrote: >>>> >>>> >>>> >>>> On Feb 8, 2016, at 8:17 PM, Duncan P. N. Exon Smith <dexonsm...@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! >
r262050. Thanks! > ~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> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits