[llvm-commits] PATCH: Additional SmallString methods

[David Blaikie]

On Fri, Jan 20, 2012 at 5:24 PM, Talin <viridia at gmail.com> wrote:
>
>
> On Fri, Jan 20, 2012 at 12:24 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> If we just made these free functions taking StringRef would that be
>> easier/more reusable? (& then relying on the conversion operator Jim
>> mentioned)
>>
>> I'd say it's a preferable design anyway (see the GoTW post on the
>> problems of the monolithic design of std::basic_string
>> http://www.gotw.ca/gotw/084.htm )
>
>
> This is primarily a stylistic argument on which I have no opinion.

Well it's a practical one in this case because if it'd been done as
free functions in the first place you wouldn't be having to do this
work now.

> I don't
> think it makes sense to re-engineer StringRef to fit with a different
> convention for how string operations are done, because the benefit isn't
> worth the cost of migration.

It's not necessarily something you'd need to migrate - you could just
provide them as free functions now & leave the StringRef member
functions as is, effectively deprecated (or as convenient
alternatives). Like so many bits of the LLVM codebase... ;)

> I'm just attempting to make all of the string
> classes within LLVM more consistent with each other and with standard STL.
> (I want to do the same with other collections as well.)

The argument from consistency is one I can't disagree with too much.
(see my recent argument from consistency for migrating ArrayRef<T> to
ArrayRef<const T> - which was shot down) Though I wouldn't mind a way
to do this while keeping the overhead lower. Could we waste a pointer
of space & have SmallString derive from StringRef? or is there some
other solution that might fit?

No big deal either way, though,
- David

>
>>
>> On Thu, Jan 19, 2012 at 11:42 PM, Talin <viridia at gmail.com> wrote:
>> > This patch adds many convenience methods to class SmallString:
>> >
>> > void assign(unsigned NumElts, char Elt);
>> > void assign(in_iter S, in_iter E);
>> > void assign(StringRef RHS);
>> > void assign(const SmallVectorImpl<char> &RHS);
>> > void append(in_iter S, in_iter E);
>> > void append(StringRef RHS);
>> > void append(const SmallVectorImpl<char> &RHS);
>> > bool equals(StringRef RHS) const;
>> > bool equals_lower(StringRef RHS) const;
>> > int compare(StringRef RHS) const;
>> > int compare_lower(StringRef RHS) const;
>> > int compare_numeric(StringRef RHS) const;
>> > bool startswith(StringRef Prefix) const;
>> > bool endswith(StringRef Suffix) const;
>> > size_t find(char C, size_t From = 0) const;
>> > size_t find(StringRef Str, size_t From = 0) const;
>> > size_t rfind(char C, size_t From = StringRef::npos) const;
>> > size_t rfind(StringRef Str) const;
>> > size_t find_first_of(char C, size_t From = 0) const;
>> > size_t find_first_of(StringRef Chars, size_t From = 0) const;
>> > size_t find_first_not_of(char C, size_t From = 0) const;
>> > size_t find_first_not_of(StringRef Chars, size_t From = 0) const;
>> > size_t find_last_of(char C, size_t From = StringRef::npos) const;
>> > size_t find_last_of(StringRef Chars, size_t From = StringRef::npos)
>> > const;
>> > size_t count(char C) const;
>> > size_t count(StringRef Str) const;
>> > StringRef substr(size_t Start, size_t N = StringRef::npos) const;
>> > StringRef slice(size_t Start, size_t End) const;
>> >
>> > Most of the methods are taken directly from StringRef (find_x,
>> > compare_x,
>> > substr, and so on). My motivation for adding these methods is that often
>> > I
>> > find myself working with a SmallString instead of a StringRef, and it
>> > would
>> > be nice to call substr() for example directly without having to convert
>> > to a
>> > StringRef. Internally, of course, the methods simply convert the
>> > SmallString
>> > to a StringRef (which is extremely cheap), and then call the
>> > corresponding
>> > StringRef method. Doing it this way minimizes template expansion, since
>> > StringRef is a non-template class.
>> >
>> > The 'assign' and 'append' methods are taken from standard STL, with
>> > additional overloads that are convenient for strings. For example,
>> > assign()
>> > now allows assigning to a SmallString from an iterator pair, which was
>> > not
>> > supported before.
>> >
>> > (Note: I thought about adding support for assign/append/construct from
>> > an
>> > ArrayRef<char>, but that would add an additional include file
>> > dependency.)
>> >
>> > This patch also includes many additions to the unit tests for
>> > SmallString,
>> > as well as some minor typographical cleanups in the comments in
>> > SmallString.h.
>> >
>> > --
>> > -- Talin
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >
>
>
>
>
> --
> -- Talin