[PATCH] A new ADT: StringRefNulTerminated

[Dmitri Gribenko]

On Thu, Feb 21, 2013 at 2:43 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Wed, Feb 20, 2013 at 4:34 PM, Sean Silva <silvas at purdue.edu> wrote:
>>
>> Overall, I think this is a good idea to have.
>>
>> Some comments about the patch:
>>
>> +    /// Construct a string ref from a string literal.
>> +    template<size_t LengthWithNul>
>> +    StringRefNulTerminated(const char (&Str)[LengthWithNul]):
>> +      StringRef(Str, LengthWithNul - 1) {}
>>
>> Is there a threat that this could end up being called by a non-string
>> literal? If so, then it should assert. I'm thinking of a scenario like
>
>
> As much fun as templated array ref ctors are - it's probably simpler just to
> take a const char*, use strlen always (you know it's null terminated - or at
> least the user is in the wrong if they pass you one that isn't) & let the
> compiler optimize away the strlen/inline the call when it's convenient to do
> so.

The motivation for this constructor is a different.  I wanted to have
an implicit conversion from a string literal, but not from any 'const
char *' pointer, since it is not uncommon in LLVM to sometimes have a
'const char *' that is not NUL-terminated -- that you just fetched
from a StringRef, or that would be put in a StringRef on the next
lines.

> That would catch cases of large buffers with small null terminated strings
> in them which the current patch doesn't handle correctly.

That is an issue indeed.  But I don't think it is common.  What is your opinion?

>> const char Foo[1] = {'a'};
>> auto X = StringRefNulTerminated(Foo);
>
>
> Like any c-string API, that's just UB. Sure, I suppose we could use the
> array-ref style thing to detect & assert on this, might be handy - but we
> don't do it for other cstring APIs so I don't see any particular reason to
> do it on this one.

I added an assert anyway -- it might be helpful, and the compiler
should optimize it out in most cases anyway.

Thank you for the review, I will post an updated patch in a few minutes.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/