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! ~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