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

[David Blaikie via llvm-commits]

(just repeating what I said on IRC for context)

My thinking was that StringLiteral would be used only for temporary
expressions (and that even the type of the global tables would be
StringRef, not StringLiteral) - so this type wouldn't end up in any APIs,
etc, keeping it fairly simple/isolated/non-intrusive/etc.

If the type is going to make its way into API usage, then I worry about
possible misuse (such as constructing it from a non-literal in one callsite
to such an API (possibly an array without a null terminator, or with a null
terminator not at the end (such as a fixed length buffer populated with a
shorter value)), even if the other started off as using it in the current
intended way).

I know lld are dabbling with a string type designed to caryr the extra
semantic info of a null terminator (StringRefZ), which might fit into all
of this somehow. (maybe StringLiteral can be converted to StringRefZ, for
example)

Or maybe I'm just off in the weeds and StringLiteral won't ever be so
constrained as to be only used in global initializers. I certainly don't
have the most context here and will defer to others who do.

On Tue, Dec 13, 2016 at 10:30 AM Zachary Turner via Phabricator <
reviews at reviews.llvm.org> wrote:

> zturner created this revision.
> zturner added reviewers: dblaikie, mehdi_amini, malcolm.parsons, chandlerc.
> zturner added a subscriber: llvm-commits.
>
> string literals are necessarily null-terminated, so it makes sense to
> provide a `c_str()` method to treat it as a null terminated string.
>
> In practice this arises because you might imagine a global table that
> contains option names which you want to pass to something like
> `getopt_long_only`, but in other situations you might want to do some
> comparisons on the option name (for example, is `--foo` in the options
> table), where you want to make use of `StringRef` comparison operators.  Of
> course, there are tons of other system calls or library calls which we
> don't have control over that take `const char*` and where we pull arguments
> for these functiosn from a global table of `const char *`, so this doesn't
> seem like just a one-off case.
>
> Writing `.str().c_str()` for these cases is problematic since it means we
> cannot assume the storage will live longer than the current statement, plus
> it's just inefficient since we're unnecessarily incurring a copy.
>
>
> https://reviews.llvm.org/D27721
>
> Files:
>   include/llvm/ADT/StringRef.h
>
>
> Index: include/llvm/ADT/StringRef.h
> ===================================================================
> --- include/llvm/ADT/StringRef.h
> +++ include/llvm/ADT/StringRef.h
> @@ -854,6 +854,8 @@
>    public:
>      template <size_t N>
>      constexpr StringLiteral(const char (&Str)[N]) : StringRef(Str, N - 1)
> {}
> +
> +    const char *c_str() const { return data(); }
>    };
>
>    /// @name StringRef Comparison Operators
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/1f57f82b/attachment.html>