[PATCH] D27686: Add llvm::StringLiteral

[Eli Friedman via Phabricator via llvm-commits]

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