[llvm] r200187 - StringRef: Extend constexpr capabilities and introduce ConstStringRef
David Blaikie
dblaikie at gmail.com
Mon Jan 27 09:40:38 PST 2014
On Sun, Jan 26, 2014 at 9:46 PM, Alp Toker <alp at nuanti.com> wrote:
> They say you should practice what you preach, and this commit had some
> changes that'll affect all users of StringRef so I've rolled it back in
> r200187 until we get some discussion going.
>
> The need derives from dozens of cases in clang where user input was being
> passed in as a format string to the diagnostic system. With all those fixed
> following ongoing work the last few months we now want a stricter check for
> compile-time fixed strings and this general solution has come together.
>
> I'll kick off with a self-review, posted inline below.. :-)
>
>
>
> On 27/01/2014 04:07, Alp Toker wrote:
>
>> Author: alp
>> Date: Sun Jan 26 22:07:17 2014
>> New Revision: 200187
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=200187&view=rev
>> Log:
>> StringRef: Extend constexpr capabilities and introduce ConstStringRef
>>
>> (1) Add llvm_expect(), an asserting macro that can be evaluated as a
>> constexpr
>> expression as well as a runtime assert or compiler hint in release
>> builds. This
>> technique can be used to construct functions that are both
>> unevaluated and
>> compiled depending on usage.
>>
>> (2) Update StringRef using llvm_expect() to preserve runtime assertions
>> while
>> extending the same checks to static asserts in C++11 builds that
>> support the
>> feature.
>>
>> (3) Introduce ConstStringRef, a strong subclass of StringRef that
>> references
>> compile-time constant strings. It's convertible to, but not from,
>> ordinary
>> StringRef and thus can be used to add compile-time safety to various
>> interfaces
>> in LLVM and clang that only accept fixed inputs such as diagnostic
>> format
>> strings that tend to get misused.
>>
>> Modified:
>> llvm/trunk/include/llvm/ADT/StringRef.h
>> llvm/trunk/include/llvm/Support/ErrorHandling.h
>> llvm/trunk/lib/Support/ErrorHandling.cpp
>> llvm/trunk/unittests/ADT/StringRefTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/ADT/StringRef.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
>> llvm/ADT/StringRef.h?rev=200187&r1=200186&r2=200187&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/include/llvm/ADT/StringRef.h (original)
>> +++ llvm/trunk/include/llvm/ADT/StringRef.h Sun Jan 26 22:07:17 2014
>> @@ -10,6 +10,8 @@
>> #ifndef LLVM_ADT_STRINGREF_H
>> #define LLVM_ADT_STRINGREF_H
>> +#include "llvm/Support/Compiler.h"
>> +#include "llvm/Support/ErrorHandling.h"
>> #include "llvm/Support/type_traits.h"
>> #include <algorithm>
>> #include <cassert>
>> @@ -70,7 +72,7 @@ namespace llvm {
>> /// @{
>> /// Construct an empty string ref.
>> - /*implicit*/ StringRef() : Data(0), Length(0) {}
>> + /*implicit*/ LLVM_CONSTEXPR StringRef() : Data(0), Length(0) {}
>> /// Construct a string ref from a cstring.
>> /*implicit*/ StringRef(const char *Str)
>> @@ -80,11 +82,8 @@ namespace llvm {
>> }
>> /// Construct a string ref from a pointer and length.
>> - /*implicit*/ StringRef(const char *data, size_t length)
>> - : Data(data), Length(length) {
>> - assert((data || length == 0) &&
>> - "StringRef cannot be built from a NULL argument with non-null
>> length");
>> - }
>> + /*implicit*/ LLVM_CONSTEXPR StringRef(const char *data, size_t
>> length)
>> + : Data(data), Length((llvm_expect(data || length == 0),
>> length)) {}
>>
>
> We lose friendly messages here, but we get back subjectively equal benefit
> by using the built-in LLVM fatal error handler instead of the one in the
> system.
>
>
> /// Construct a string ref from an std::string.
>> /*implicit*/ StringRef(const std::string &Str)
>> @@ -104,24 +103,20 @@ namespace llvm {
>> /// data - Get a pointer to the start of the string (which may
>> not be null
>> /// terminated).
>> - const char *data() const { return Data; }
>> + LLVM_CONSTEXPR const char *data() const { return Data; }
>> /// empty - Check if the string is empty.
>> - bool empty() const { return Length == 0; }
>> + LLVM_CONSTEXPR bool empty() const { return Length == 0; }
>> /// size - Get the string size.
>> - size_t size() const { return Length; }
>> + LLVM_CONSTEXPR size_t size() const { return Length; }
>> /// front - Get the first character in the string.
>> - char front() const {
>> - assert(!empty());
>> - return Data[0];
>> - }
>> + LLVM_CONSTEXPR char front() const { return llvm_expect(!empty()),
>> Data[0]; }
>> /// back - Get the last character in the string.
>> - char back() const {
>> - assert(!empty());
>> - return Data[Length-1];
>> + LLVM_CONSTEXPR char back() const {
>> + return llvm_expect(!empty()), Data[Length - 1];
>> }
>> /// equals - Check for string equality, this is more efficient
>> than
>> @@ -187,9 +182,8 @@ namespace llvm {
>> /// @name Operator Overloads
>> /// @{
>> - char operator[](size_t Index) const {
>> - assert(Index < Length && "Invalid index!");
>> - return Data[Index];
>> + LLVM_CONSTEXPR char operator[](size_t Index) const {
>> + return llvm_expect(Index < Length), Data[Index];
>> }
>>
>
> Ditto.
>
>
> /// @}
>> @@ -547,6 +541,20 @@ namespace llvm {
>> /// @}
>> + /// ConstStringRef - A \c StringRef carrying the additional
>> stipulation that
>> + /// the referenced string is a compile-time constant.
>> + ///
>> + /// Use this to specify function parameters that require fixed inputs
>> such
>> + /// as debug and diagnostic messages or format strings.
>> + class ConstStringRef : public StringRef {
>> + public:
>> + /*implicit*/ LLVM_CONSTEXPR ConstStringRef() : StringRef() {}
>> +
>> + template <size_t N>
>> + /*implicit*/ LLVM_CONSTEXPR ConstStringRef(const char (&data)[N])
>> + : StringRef(data, (llvm_expect(N > 0 && data[N - 1] == '\0'), N
>> - 1)) {}
>>
>
> This constructor doesn't technically require a constant string literal and
> you can trick it into accepting a fixed-length char array, but in all uses
> it detected errors that have been fixed.
Could you point to revisions that demonstrate the fixes that have been
prompted by this?
> There's some interest in introducing a clang attribute to check for
> literals and that would get added here as the feature evolves.
>
>
> + };
>> +
>> /// \brief Compute a hash_code for a StringRef.
>> hash_code hash_value(StringRef S);
>>
>> Modified: llvm/trunk/include/llvm/Support/ErrorHandling.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
>> llvm/Support/ErrorHandling.h?rev=200187&r1=200186&r2=200187&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/include/llvm/Support/ErrorHandling.h (original)
>> +++ llvm/trunk/include/llvm/Support/ErrorHandling.h Sun Jan 26 22:07:17
>> 2014
>> @@ -15,11 +15,11 @@
>> #ifndef LLVM_SUPPORT_ERRORHANDLING_H
>> #define LLVM_SUPPORT_ERRORHANDLING_H
>> -#include "llvm/ADT/StringRef.h"
>> #include "llvm/Support/Compiler.h"
>> #include <string>
>> namespace llvm {
>> + class StringRef;
>> class Twine;
>> /// An error handler callback.
>> @@ -78,7 +78,7 @@ namespace llvm {
>> bool gen_crash_diag =
>> true);
>> LLVM_ATTRIBUTE_NORETURN void report_fatal_error(const std::string
>> &reason,
>> bool gen_crash_diag =
>> true);
>> - LLVM_ATTRIBUTE_NORETURN void report_fatal_error(StringRef reason,
>> + LLVM_ATTRIBUTE_NORETURN void report_fatal_error(const StringRef
>> &reason,
>> bool gen_crash_diag =
>> true);
>>
>
> This tweak drops the early dependency on StringRef.h so we can use
> ErrorHandling in StringRef itself.
>
>
>
> LLVM_ATTRIBUTE_NORETURN void report_fatal_error(const Twine &reason,
>> bool gen_crash_diag =
>> true);
>> @@ -108,4 +108,14 @@ namespace llvm {
>> #define llvm_unreachable(msg) ::llvm::llvm_unreachable_internal()
>> #endif
>> +/// An assert macro that's usable in constexprs and that becomes an
>> optimizer
>> +/// hint in NDEBUG builds.
>> +///
>> +/// Unlike \c assert() the \param test expression may be evaluated in
>> optimized
>> +/// builds and so should be simple, accurate and never have side effects.
>> +#define llvm_expect(test) (void)(!!(test) ? 0 :
>> (llvm_unreachable(#test), 0))
>>
>
> This macro expansion could do with checking over. If it works out
> llvm_expect() will have better characteristics in debug builds.
>
> The optimizer hint approach is something to consider. We have to evaluate
> 'test' to support constexpr evaluation but we don't strictly need to use it
> as a hint if llvm_unreachable() feels too strong here.
>
> Alp.
>
>
>
> +
>> +// TODO: Update other headers to explicitly include StringRef.h and drop
>> this.
>> +#include "llvm/ADT/StringRef.h"
>> +
>> #endif
>>
>> Modified: llvm/trunk/lib/Support/ErrorHandling.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
>> Support/ErrorHandling.cpp?rev=200187&r1=200186&r2=200187&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Support/ErrorHandling.cpp (original)
>> +++ llvm/trunk/lib/Support/ErrorHandling.cpp Sun Jan 26 22:07:17 2014
>> @@ -58,7 +58,7 @@ void llvm::report_fatal_error(const std:
>> report_fatal_error(Twine(Reason), GenCrashDiag);
>> }
>> -void llvm::report_fatal_error(StringRef Reason, bool GenCrashDiag) {
>> +void llvm::report_fatal_error(const StringRef &Reason, bool
>> GenCrashDiag) {
>> report_fatal_error(Twine(Reason), GenCrashDiag);
>> }
>>
>> Modified: llvm/trunk/unittests/ADT/StringRefTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/
>> ADT/StringRefTest.cpp?rev=200187&r1=200186&r2=200187&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/unittests/ADT/StringRefTest.cpp (original)
>> +++ llvm/trunk/unittests/ADT/StringRefTest.cpp Sun Jan 26 22:07:17 2014
>> @@ -531,4 +531,22 @@ TEST(StringRefTest, joinStrings) {
>> EXPECT_TRUE(v2_join3);
>> }
>> +static void fn_stringref(StringRef str) {
>> + EXPECT_TRUE(str == "hello");
>> +}
>> +static void fn_conststringref(ConstStringRef str) {
>> + fn_stringref(str);
>> +}
>> +
>> +TEST(StringRefTest, constStringRef) {
>> + LLVM_CONSTEXPR ConstStringRef csr("hello");
>> +#if __has_feature(cxx_constexpr) || defined(__GXX_EXPERIMENTAL_CXX0X__)
>> + LLVM_STATIC_ASSERT(csr[0] != csr[1], "");
>> + LLVM_STATIC_ASSERT(csr[2] == csr[3], "");
>> + LLVM_STATIC_ASSERT(csr.size() == 5, "");
>> +#endif
>> + llvm_expect(csr[2] == csr[3]);
>> + fn_conststringref(csr);
>> +}
>> +
>> } // end anonymous namespace
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> --
> http://www.nuanti.com
> the browser experts
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140127/b1059c0d/attachment.html>
More information about the llvm-commits
mailing list