patch: new attribute enable_if
nicholas at mxc.ca
Fri Jan 10 03:11:39 PST 2014
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.
> 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
Done with remove_if + reverse. It's an improvement.
One other change, I updated the docs to not use __builtin_constant_p
void foo(int i) __attribute__((enable_if(__builtin_constant_p(i))));
does not actually evaluate __builtin_constant_p(expr).
Updated patch attached.
> 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-
> 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 >
> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 45196 bytes
Desc: not available
More information about the cfe-commits