patch: new attribute enable_if

Nick Lewycky nicholas at mxc.ca
Mon Dec 30 20:27:40 PST 2013


This is the continuation of this thread:

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-4.patch
Type: text/x-diff
Size: 45530 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131230/eda49ff6/attachment.patch>


More information about the cfe-commits mailing list