patch: new attribute enable_if

[Nick Lewycky]

Richard Smith wrote:
> This looks really good. Just a few things:
>
>
> In LanguageExtensions.rst:
>
> "In this way, we pick the more specific"
>
> ... missing end of sentence.

Fixed.

> SemaDeclAttr.cpp: can the Subject and number-of-arguments checking be
> handled by the common attribute-handling code?

Turns out it can! Done.

> CheckEnableIf: can you perform the parameter initialization/conversion
> steps once, rather than once per enable_if attribute? Also, perhaps use
> std::remove_if rather than the current O(n^2) loop to extract the
> enable_if attributes (here and later in the same file), or maybe
> directly iterate over the attribute lists without the additional
> copy/filter?

Done with remove_if + reverse. It's an improvement.

One other change, I updated the docs to not use __builtin_constant_p 
either. This:
   void foo(int i) __attribute__((enable_if(__builtin_constant_p(i))));
   ...
   foo(expr)
does not actually evaluate __builtin_constant_p(expr).

Updated patch attached.

Nick

> On Mon, Dec 30, 2013 at 8:27 PM, Nick Lewycky <nicholas at mxc.ca
> <mailto:nicholas at mxc.ca>> wrote:
>
>     This is the continuation of this thread:
>
>     http://lists.cs.uiuc.edu/ pipermail/cfe-commits/Week-of-
>     Mon-20131007/090357.html
>     <http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131007/090357.html>
>
>     from October 7.
>
>     This update to the patch includes a significant change in semantics.
>     We now support partial ordering of candidates when more than one
>     enable_if attribute is applied to a declaration. The exact semantics
>     of this feature are described in the LanguageExtensions.rst portion
>     of the patch, instead I want to talk about the rationale.
>
>     With the previous form of enable_if, overload resolution would be
>     ambiguous unless all but one expression folded to false. Suppose you
>     have four properties to test:
>        p1) nothing is known, call regular strnlen
>        p2) array length is known and maxlen isn't known, call strnlen_chk
>        p3) array length is known and maxlen is known and it fits
>        p4) array length is known and maxlen is known and it doesn't fit
>
>     The four function declarations would need to each test all four of
>     those properties. That's a little ugly, but there's one gotcha that
>     makes it awful: an enable_if expression is a tristate, it could be
>     true, false or could-not-compute (which is observed as false). This
>     means that simply writing __attribute__((enable_if(p1 && !p2 &&
>     ...))) doesn't work.
>
>     Instead, we handled this case by making user call
>     __builtin_constant_p() in their enable_if expression to check
>     whether a given argument was a constant, but then that necessitated
>     changes to BCP's constant folding, changes that may make BCP behave
>     differently from GCC.
>
>     The new approach is to add partial function ordering semantics based
>     on the rules for partial ordering of function templates
>     [temp.func.order]. It's the same as "function with largest number of
>     enable_if attributes wins" except that if the n-th enable_if
>     expression on two candidates are not ODR-equivalent, then the two
>     are ambiguous regardless of who has more enable_if attributes.
>
>     This allows us to implement our four property case with:
>        i1) a base case with no enable_if attributes
>        i2) enable_if(__builtin_object_ size(s) != -1)
>        i3) enable_if(__builtin_object_ size(s) != -1) enable_if(maxlen
>     <= __builtin_object_size(s))
>        i4) enable_if(__builtin_object_ size(s) != -1) enable_if(maxlen >
>     __builtin_object_size(s))
>
>     In particular, all the tests from the previous version of this patch
>     have been updated to use multiple enable_if attributes where
>     necessary, and none of them use __builtin_constant_p any more. Victory!
>
>     Well, let's not declare victory yet. The implementation has some
>     truly nasty stuff as seen in the patch. The major one is that I
>     can't use specific_attr_iterator in two places I want to because it
>     doesn't guarantee the correct ordering. The replacement code is akin
>     to a lovecraftian horror. Yes, I want to commit it like this first
>     and fix it later.
>
>     I'm also not completely confident in my changes to ExprConstant.cpp.
>     Does adding new EvalModes make sense for this? What is the
>     difference between EM_ConstantExpression and EM_ConstantFold, and
>     could I have made use of that instead of adding new modes?
>
>     Please review. I'd appreciate making note of which review comments
>     block submission, and which can be incremental improvements after
>     the initial commit.
>
>     Nick
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: enable_if-5.patch
Type: text/x-diff
Size: 45196 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140110/ba01108b/attachment.patch>