[llvm] r200187 - StringRef: Extend constexpr capabilities and introduce ConstStringRef
Alp Toker
alp at nuanti.com
Sun Jan 26 21:46:52 PST 2014
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. 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
More information about the llvm-commits
mailing list