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

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 14 17:02:20 PST 2016

It catches two cases:

1) __builtin_strlen() doesn't even work at compile time (because it's
variable length)
2) __builtin_strlen() does work at compile time, but is not equal to
sizeof(Array) - 1).  This catches the case where you have an embedded null
 (thereby matching the semantics of StringRef("abc\0"))

On Wed, Dec 14, 2016 at 4:59 PM David Blaikie <dblaikie at gmail.com> wrote:

> 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/057f375e/attachment.html>

More information about the llvm-commits mailing list