[PATCH] A new ADT: StringRefNulTerminated

[Dmitri Gribenko]

Hi Sean,

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

On Thu, Feb 21, 2013 at 2:34 AM, 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
>
> const char Foo[1] = {'a'};
> auto X = StringRefNulTerminated(Foo);

Done.

>
> +  /// \brief Represents a constant reference to a NUL-terminated string.
> +  /// It can be safely converted to 'const char *' without copying the
> +  /// underlying data.
> +  class StringRefNulTerminated : public StringRef {
> +  public:
>
> This comment should really say something explaining that "it is just a
> way to represent in the type system that 'one past the end' of the
> StringRef is valid memory guaranteed to be NUL". The discussion in the
> rst documentation could be significantly simplified by describing it
> like this as well.

Changed the comment.

> +  /// It can be safely converted to 'const char *' without copying the
> +  /// underlying data.
>
> You should make clear that this is the motivation e.g. "The motivation
> is that it can be safely ...". I would also drop "safely" to avoid
> even suggesting the idea of looking one-past-the-end of a regular
> StringRef.

A good idea.  I was using 'safely' because I was fixing that
one-past-end read, but it should not have been written.

> +    StringRefNulTerminated(const char *Data, size_t Length):
> +        StringRef(Data, Length) {
> +      assert(Data[Length] == '\0' && "");
>
> Please make the message actually helpful. When someone crashes on this
> assert they want something meaningful.

Done.  I actually intended to write the message, but apparently forgot.

-- 
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>*/