[PATCH] D27686: Add llvm::StringLiteral

[David Blaikie via llvm-commits]

Ping on Eli's feedback - it sounds plausible to me.

On Tue, Dec 13, 2016 at 10:38 AM Eli Friedman via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> efriedma added inline comments.
>
>
> ================
> Comment at: include/llvm/ADT/StringRef.h:90
> -        assert((data || length == 0) &&
> -        "StringRef cannot be built from a NULL argument with non-null
> length");
> -      }
> ----------------
> zturner wrote:
> > efriedma wrote:
> > > mehdi_amini wrote:
> > > > Too bad we're losing the assert, but I see why. An alternative is
> another (possibly protected) ctor that would be constexpr and used by
> `StringLiteral`
> > > Stupid trick to save the assert:
> > >
> > >     /*implicit*/ constexpr StringRef(const char *data, long length)
> > >         : Data(data), Length((data || length == 0) ? length :
> (assert(0 && "Bad StringRef"), 0)) { }
> > >
> > >
> > Won't this still complain that `assert` is not a constant expression and
> thus can't be used in a `constexpr` function?
> No... formally, [expr.const] in the C++11 standard says that when checking
> whether an expression is a constant expression, "conditional operations
> that are not evaluated are not considered".  In practice, this means you'll
> get an error at compile time only if the condition is false, which is
> exactly the behavior you want.
>
> I actually screwed up the implementation slightly; it's not a no-op if
> assertions are turned off, but that's easy to fix:
>
>     /*implicit*/ constexpr StringRef(const char *data, long length)
>         : Data(data), Length((data || length == 0) ? length : (assert(0 &&
> "Bad StringRef"), length)) { }
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D27686
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161220/9ae34859/attachment-0001.html>