headers
Andy Gibbs | 21 Jun 2012 18:58
Picon
Favicon

Patch for review: enhancement to __builtin_strlen

Hi,

I would like to offer a patch for review that extends the use of the 
compile-time evaluation of __builtin_strlen to include immutable 
null-terminated const char arrays, i.e. beyond just standard string 
literals.  (It will still fall back to runtime use of library strlen, if 
compile-time evaluation is not possible/required -- this side of the 
functionality is not changed).

This is not a bug fix but a feature enhancement, which I believe may have 
broader appeal to those developing with C++11 and, in particular, with 
user-defined literals.

The target use for this would be in generating transparent wrapper classes 
for user-defined literals, so that these may be used in situations wherever 
normal strings may be used.

Let's say we have the following simplified setup:

template <char... String>
struct Literal {
  private:
  typedef const char type[sizeof...(String)+1];
  static constexpr type string = { String..., 0 };

  public:
  constexpr operator const type&() const { return string; }

  // room for more implementation, such as operator[] and
  // also string manipulation, such as operator+, and so on.
(Continue reading)

Permalink | Reply |
headers
Richard Smith | 22 Jun 2012 00:13
Picon

Re: Patch for review: enhancement to __builtin_strlen

Hi,

On Thu, Jun 21, 2012 at 9:58 AM, Andy Gibbs <andyg1001@...> wrote:
> I would like to offer a patch for review that extends the use of the
> compile-time evaluation of __builtin_strlen to include immutable
> null-terminated const char arrays, i.e. beyond just standard string
> literals.  (It will still fall back to runtime use of library strlen, if
> compile-time evaluation is not possible/required -- this side of the
> functionality is not changed).
>
> This is not a bug fix but a feature enhancement, which I believe may have
> broader appeal to those developing with C++11 and, in particular, with
> user-defined literals.

This seems like a completely reasonable enhancement. As we inevitably
shift towards making more library functions usable in constant
expressions, this seems very much like a feature we will want.

> The patch is intended to be very conservative in terms of what it will
> accept, and so the extension only allows constant sized arrays of const char
> (__builtin_strlen limits this to char, although wchar_t, etc. would work
> otherwise) and where the compiler can evaluate the expression as a constant
> array (i.e. not a pointer or an lvalue of an array that can be modified at
> runtime, for example).  I believe there are adequate checks to stop
> incorrect compilation, for example, incorrectly doing a compile-time
> evaluation of __builtin_strlen on a variable which is not immutable, but
> please can this be particularly verified by someone.

I think the patch is actually too conservative. For instance, I would
like for the following to work:
(Continue reading)

Permalink | Reply |
headers
Andy Gibbs | 23 Jun 2012 00:46
Picon
Favicon

Re: Patch for review: enhancement to __builtin_strlen

On Friday, June 22, 2012 12:13 AM, Richard Smith wrote:
> I think the patch is actually too conservative. For instance, I would
> like for the following to work:
>
>   constexpr const char *p = "foobar";
>   constexpr int n = __builtin_strlen(p);
>
> More generally, the builtin should behave as if it were defined as:
>
>  constexpr size_t __builtin_strlen(const char *p) { return *p ? 1 +
> __builtin_strlen(p+1) : 0; }
>
> ... except that it should be faster (and not limited by the constexpr
> recursion depth limit).

Ok, attached then is a first patch towards this end.  It works in most
scenarios, and I have found only one real place where it doesn't, which
is using __builtin_strlen inside a constexpr function with a function
parameter as the expression, but I am pretty sure this is not a fault
in my implementation, but rather that __builtin_strlen as it currently
stands doesn't work inside constexpr functions.  I'll look into it.

I'd like to see if we can get this first patch committed, then I can
look at doing the next stage (otherwise, I just end up with massive
sets of patches that never get committed!...)

I haven't followed exactly your suggestions, but maybe you will spot
some of them in the code anyway.  I actually made the bold step of
splitting out the functionality into its own function, namely
Expr::EvaluateAsString which then returns a folded, truncated
(Continue reading)

Permalink | Reply |
headers
Richard Smith | 23 Jun 2012 01:34
Picon

Re: Patch for review: enhancement to __builtin_strlen

On Fri, Jun 22, 2012 at 3:46 PM, Andy Gibbs <andyg1001@...> wrote:
> On Friday, June 22, 2012 12:13 AM, Richard Smith wrote:
>> I think the patch is actually too conservative. For instance, I would
>> like for the following to work:
>>
>>   constexpr const char *p = "foobar";
>>   constexpr int n = __builtin_strlen(p);
>>
>> More generally, the builtin should behave as if it were defined as:
>>
>>  constexpr size_t __builtin_strlen(const char *p) { return *p ? 1 +
>> __builtin_strlen(p+1) : 0; }
>>
>> ... except that it should be faster (and not limited by the constexpr
>> recursion depth limit).
>
> Ok, attached then is a first patch towards this end.  It works in most
> scenarios, and I have found only one real place where it doesn't, which
> is using __builtin_strlen inside a constexpr function with a function
> parameter as the expression, but I am pretty sure this is not a fault
> in my implementation, but rather that __builtin_strlen as it currently
> stands doesn't work inside constexpr functions.  I'll look into it.

This will be because you're still using Expr::EvaluateAsRValue: that
won't work for references to constexpr function parameters, because it
doesn't operate in the same EvalInfo context.

> I'd like to see if we can get this first patch committed, then I can
> look at doing the next stage (otherwise, I just end up with massive
> sets of patches that never get committed!...)
(Continue reading)

Permalink | Reply |
headers
Andy Gibbs | 23 Jun 2012 23:01
Picon
Favicon

Re: Patch for review: enhancement to __builtin_strlen

On Saturday, June 23, 2012 1:34 AM, Richard Smith wrote:
> On Fri, Jun 22, 2012 at 3:46 PM, Andy Gibbs <andyg1001@...> 
> wrote:
>> Ok, attached then is a first patch towards this end.  It works in most
>> scenarios, and I have found only one real place where it doesn't, which
>> is using __builtin_strlen inside a constexpr function with a function
>> parameter as the expression, but I am pretty sure this is not a fault
>> in my implementation, but rather that __builtin_strlen as it currently
>> stands doesn't work inside constexpr functions.  I'll look into it.
>
> This will be because you're still using Expr::EvaluateAsRValue: that
> won't work for references to constexpr function parameters, because it
> doesn't operate in the same EvalInfo context.

Ok, I didn't realise this was the reason -- I'm still learning how the code 
works!

>> I'd like to see if we can get this first patch committed, then I can
>> look at doing the next stage (otherwise, I just end up with massive
>> sets of patches that never get committed!...)
>>
>> I haven't followed exactly your suggestions, but maybe you will spot
>> some of them in the code anyway.  I actually made the bold step of
>> splitting out the functionality into its own function, namely
>> Expr::EvaluateAsString which then returns a folded, truncated
>> std::string result on success.  __builtin_strlen then calls this
>> and returns the length of this string.
>
> This approach has the downside of copying the string even when it's a
> string literal. This construct appears pretty frequently in code using
(Continue reading)

Permalink | Reply |

Gmane