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

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 13 11:29:41 PST 2016


> On Dec 13, 2016, at 10:36 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> (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 of the global is StringRef, doesn’t this make it invoking the copy ctor from StringRef when initializing from StringLiteral? Would this still be constexpr?

> 
> 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 <mailto: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 <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/090b86d7/attachment-0001.html>


More information about the llvm-commits mailing list