[PATCH] D27721: Add a c_str() member to StringLiteral

[David Blaikie via llvm-commits]

Sounds OK to me - though I don't fully understand which cases it
catches/doesn't/etc. (that's OK, really :) )

On Wed, Dec 14, 2016 at 4:53 PM Zachary Turner <zturner at google.com> wrote:

> Yea, that's fine.  I still think the attribute should go in regardless, as
> it prevents the case of creating a StringLiteral with a char array.  Agree
> / disagree?
>
> On Wed, Dec 14, 2016 at 4:35 PM David Blaikie <dblaikie at gmail.com> wrote:
>
> On Wed, Dec 14, 2016 at 3:55 PM Zachary Turner via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
> zturner added a comment.
>
> Probably should take this discussion to a thread specifically about
> `StringRefZ`, but fwiw I think that would complicate the logic of
> `StringRef` considerably while also introducing thread synchronization
> issues and potentially increasing the size of a `StringRef`.  It's possible
> seeing the actual implementation would change my mind, but so far it
> doesn't sound like a good candidate for `StringRef`.
>
> Back to this patch, anyone have any other thoughts?  FWIW I changed my
> usage so I'm now storing a `StringRef` instead of a `StringLiteral`
> (because it actually needed to be constructed without a literal in some
> scenarios), but the API still seems to make sense here to me (and the
> attribute seems like a good addition regardless)
>
>
> If you've changed your use case to use StringRef now - do you have a
> use/need for this feature now?
>
> Otherwise I'd probably wait until there's a use case to discuss it in
> context, etc. (the patch seems small enough that it's not like we're losing
> a lot of technical work to create it again next time we come up with a case
> that might benefit)
>
>
>
>
> https://reviews.llvm.org/D27721
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161215/228ef5db/attachment.html>