Explicit conversion of SmallString to std::string

[Yaron Keren]

http://reviews.llvm.org/D6336
(Not ready yet)


2014-10-15 23:31 GMT+03:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On Oct 15, 2014, at 9:18 AM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Thanks for chiming in, Duncan. Mostly just agreeing with what you've
> said here.
>
>
> > Are there any tradeoffs to consider between StringRef providing an
> implicit ctor from SmallString, compared to SmallString providing an
> implicit conversion to StringRef (which it already has)?
> >
>
> > What is possible with a StringRef(SmallString) ctor that's not possible
> (or convenient) with 'SmallString::operator StringRef'?
>
> Hmm... I think I've hit cases where implicit conversions cause ambiguity
> that implicit ctors don't, but I can't come up with an example right
> now.  Maybe that's just dogma from my past?
>
> > The only minor wrinkle is that this ctor will not be inline, because
> SmallString does really depend on StringRef (it has lots of functions that
> reasonably take StringRef by value, reference StringRef::npos (well, we
> could pull that out to some generic utility I suppose), etc) - so
> StringRef.h can have a forward declaration of SmallString which it can use
> to declare a StringRef(const SmallString&) ctor, then an out-of-line
> definition of that in StringRef.cpp.
>
> We can add a constructor from `SmallVectorImpl<char>`, `SmallString`'s
> parent class.  This makes `StringRef` constructible from
> `SmallVector<char>` as well; I'm not sure whether that's a feature or a
> defect, but it seems reasonable to me.
>
> Or, we could use a cross-include:
>
>     $ cat StringRef.h
>     namespace llvm {
>     class StringRef {
>     public:
>       inline StringRef(const SmallString &S);
>       /* ... */
>     };
>     }
>     #include "llvm/ADT/SmallString.h"
>
>     $ cat SmallString.h
>     #include "llvm/Support/StringRef.h"
>     namespace llvm {
>     class SmallString { /* ... */ };
>     StringRef::StringRef(const SmallString &S) { /* ... */ }
>     }
>
> Or, if my aversion to implicit conversion operators is just religious,
> then we can just focus on fixing `SmallStr::str()` :).
>
> > I agree with David's original point here.  I think `.str()` should just
> > create a `std::string`.  I'd stage it like this:
> >
> >  1. Add a `StringRef::StringRef(SmallString)` constructor and remove
> >     the `SmallString::operator StringRef()` conversion.
> >
> >  2. Incrementally remove uses of `SmallString::str()`.
> >
> >  3. Remove `SmallString::str()` entirely once it's dead code.
> >
> >  4. After waiting a couple of days as a courtesy to be sure out-of-tree
> >     code has been updated, add back `SmallString::str()` with new
> >     semantics.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141120/e23cd778/attachment.html>