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. 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?

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

Reply via email to