Skip to content

Commit

Permalink
Make base::StringPiece more constexpr
Browse files Browse the repository at this point in the history
The following methods have been made constexpr:
- the implicit conversion constructor base::StringPiece(const value_type*)
- the comparison operators and base::StringPiece::compare()
- base::StringPiece::starts_with() and base::StringPiece::ends_with()

Since std::char_traits<> isn't constexpr-ified in C++14, this CL also
implements base::CharTraits<> to provide constexpr versions for the
base::StringPiece implementation.

Finally, this also updates a few callers in Chrome to take advantage of
the new constexpr functionality.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_mojo
Change-Id: Id3cf2be14bfb5adb53a5ca1bd74ec2a48e411f60
Reviewed-on: https://chromium-review.googlesource.com/1038746
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556258}
  • Loading branch information
zetafunction authored and Commit Bot committed May 4, 2018
1 parent ce15d2f commit 154c940
Show file tree
Hide file tree
Showing 9 changed files with 260 additions and 50 deletions.
2 changes: 2 additions & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,7 @@ jumbo_component("base") {
"sha1.h",
"single_thread_task_runner.h",
"stl_util.h",
"strings/char_traits.h",
"strings/latin1_string_conversions.cc",
"strings/latin1_string_conversions.h",
"strings/nullable_string16.cc",
Expand Down Expand Up @@ -2306,6 +2307,7 @@ test("base_unittests") {
"sequenced_task_runner_unittest.cc",
"sha1_unittest.cc",
"stl_util_unittest.cc",
"strings/char_traits_unittest.cc",
"strings/nullable_string16_unittest.cc",
"strings/pattern_unittest.cc",
"strings/safe_sprintf_unittest.cc",
Expand Down
90 changes: 90 additions & 0 deletions base/strings/char_traits.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef BASE_STRINGS_CHAR_TRAITS_H_
#define BASE_STRINGS_CHAR_TRAITS_H_

#include <stddef.h>

namespace base {

// constexpr version of http://en.cppreference.com/w/cpp/string/char_traits.
// This currently just implements the bits needed to support a (mostly)
// constexpr StringPiece.
//
// TODO(dcheng): Once we switch to C++17, most methods will become constexpr and
// we can switch over to using the one in the standard library.
template <typename T>
struct CharTraits {
// Performs a lexographical comparison of the first N characters of |s1| and
// |s2|. Returns 0 if equal, -1 if |s1| is less than |s2|, and 1 if |s1| is
// greater than |s2|.
static constexpr int compare(const T* s1, const T* s2, size_t n) noexcept;

// Returns the length of |s|, assuming null termination (and not including the
// terminating null).
static constexpr size_t length(const T* s) noexcept;
};

template <typename T>
constexpr int CharTraits<T>::compare(const T* s1,
const T* s2,
size_t n) noexcept {
for (; n; --n, ++s1, ++s2) {
if (*s1 < *s2)
return -1;
if (*s1 > *s2)
return 1;
}
return 0;
}

template <typename T>
constexpr size_t CharTraits<T>::length(const T* s) noexcept {
size_t i = 0;
for (; *s; ++s)
++i;
return i;
}

// char specialization of CharTraits that can use clang's constexpr instrinsics,
// where available.
template <>
struct CharTraits<char> {
static constexpr int compare(const char* s1,
const char* s2,
size_t n) noexcept;
static constexpr size_t length(const char* s) noexcept;
};

constexpr int CharTraits<char>::compare(const char* s1,
const char* s2,
size_t n) noexcept {
#if __has_feature(cxx_constexpr_string_builtins)
return __builtin_memcmp(s1, s2, n);
#else
for (; n; --n, ++s1, ++s2) {
if (*s1 < *s2)
return -1;
if (*s1 > *s2)
return 1;
}
return 0;
#endif
}

constexpr size_t CharTraits<char>::length(const char* s) noexcept {
#if defined(__clang__)
return __builtin_strlen(s);
#else
size_t i = 0;
for (; *s; ++s)
++i;
return i;
#endif
}

} // namespace base

#endif // BASE_STRINGS_CHAR_TRAITS_H_
32 changes: 32 additions & 0 deletions base/strings/char_traits_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/strings/char_traits.h"
#include "base/strings/string16.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace base {

TEST(CharTraitsTest, CharCompare) {
static_assert(CharTraits<char>::compare("abc", "def", 3) == -1, "");
static_assert(CharTraits<char>::compare("def", "def", 3) == 0, "");
static_assert(CharTraits<char>::compare("ghi", "def", 3) == 1, "");
}

TEST(CharTraitsTest, CharLength) {
static_assert(CharTraits<char>::length("") == 0, "");
static_assert(CharTraits<char>::length("abc") == 3, "");
}

TEST(CharTraitsTest, Char16TCompare) {
static_assert(CharTraits<char16_t>::compare(u"abc", u"def", 3) == -1, "");
static_assert(CharTraits<char16_t>::compare(u"def", u"def", 3) == 0, "");
static_assert(CharTraits<char16_t>::compare(u"ghi", u"def", 3) == 1, "");
}

TEST(CharTraitsTest, Char16TLength) {
static_assert(CharTraits<char16_t>::length(u"abc") == 3, "");
}

} // namespace base
3 changes: 2 additions & 1 deletion base/strings/string_piece.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ bool operator==(const StringPiece& x, const StringPiece& y) {
if (x.size() != y.size())
return false;

return StringPiece::wordmemcmp(x.data(), y.data(), x.size()) == 0;
return CharTraits<StringPiece::value_type>::compare(x.data(), y.data(),
x.size()) == 0;
}

std::ostream& operator<<(std::ostream& o, const StringPiece& piece) {
Expand Down
41 changes: 21 additions & 20 deletions base/strings/string_piece.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include "base/base_export.h"
#include "base/logging.h"
#include "base/strings/char_traits.h"
#include "base/strings/string16.h"
#include "base/strings/string_piece_forward.h"

Expand Down Expand Up @@ -176,9 +177,12 @@ template <typename STRING_TYPE> class BasicStringPiece {
// in a "const char*" or a "string" wherever a "StringPiece" is
// expected (likewise for char16, string16, StringPiece16).
constexpr BasicStringPiece() : ptr_(NULL), length_(0) {}
BasicStringPiece(const value_type* str)
: ptr_(str),
length_((str == NULL) ? 0 : STRING_TYPE::traits_type::length(str)) {}
// TODO(dcheng): Construction from nullptr is not allowed for
// std::basic_string_view, so remove the special handling for it.
// Note: This doesn't just use STRING_TYPE::traits_type::length(), since that
// isn't constexpr until C++17.
constexpr BasicStringPiece(const value_type* str)
: ptr_(str), length_(!str ? 0 : CharTraits<value_type>::length(str)) {}
BasicStringPiece(const STRING_TYPE& str)
: ptr_(str.data()), length_(str.size()) {}
constexpr BasicStringPiece(const value_type* offset, size_type len)
Expand Down Expand Up @@ -245,8 +249,8 @@ template <typename STRING_TYPE> class BasicStringPiece {
length_ -= n;
}

int compare(const BasicStringPiece<STRING_TYPE>& x) const {
int r = wordmemcmp(
constexpr int compare(BasicStringPiece x) const noexcept {
int r = CharTraits<value_type>::compare(
ptr_, x.ptr_, (length_ < x.length_ ? length_ : x.length_));
if (r == 0) {
if (length_ < x.length_) r = -1;
Expand Down Expand Up @@ -275,12 +279,6 @@ template <typename STRING_TYPE> class BasicStringPiece {
size_type max_size() const { return length_; }
size_type capacity() const { return length_; }

static int wordmemcmp(const value_type* p,
const value_type* p2,
size_type N) {
return STRING_TYPE::traits_type::compare(p, p2, N);
}

// Sets the value of the given string target type to be the current string.
// This saves a temporary over doing |a = b.as_string()|
void CopyToString(STRING_TYPE* target) const {
Expand All @@ -296,16 +294,18 @@ template <typename STRING_TYPE> class BasicStringPiece {
}

// Does "this" start with "x"
bool starts_with(const BasicStringPiece& x) const {
return ((this->length_ >= x.length_) &&
(wordmemcmp(this->ptr_, x.ptr_, x.length_) == 0));
constexpr bool starts_with(BasicStringPiece x) const noexcept {
return (
(this->length_ >= x.length_) &&
(CharTraits<value_type>::compare(this->ptr_, x.ptr_, x.length_) == 0));
}

// Does "this" end with "x"
bool ends_with(const BasicStringPiece& x) const {
constexpr bool ends_with(BasicStringPiece x) const noexcept {
return ((this->length_ >= x.length_) &&
(wordmemcmp(this->ptr_ + (this->length_-x.length_),
x.ptr_, x.length_) == 0));
(CharTraits<value_type>::compare(
this->ptr_ + (this->length_ - x.length_), x.ptr_, x.length_) ==
0));
}

// find: Search for a character or substring at a given offset.
Expand Down Expand Up @@ -395,7 +395,7 @@ inline bool operator!=(const StringPiece& x, const StringPiece& y) {
}

inline bool operator<(const StringPiece& x, const StringPiece& y) {
const int r = StringPiece::wordmemcmp(
const int r = CharTraits<StringPiece::value_type>::compare(
x.data(), y.data(), (x.size() < y.size() ? x.size() : y.size()));
return ((r < 0) || ((r == 0) && (x.size() < y.size())));
}
Expand All @@ -418,15 +418,16 @@ inline bool operator==(const StringPiece16& x, const StringPiece16& y) {
if (x.size() != y.size())
return false;

return StringPiece16::wordmemcmp(x.data(), y.data(), x.size()) == 0;
return CharTraits<StringPiece16::value_type>::compare(x.data(), y.data(),
x.size()) == 0;
}

inline bool operator!=(const StringPiece16& x, const StringPiece16& y) {
return !(x == y);
}

inline bool operator<(const StringPiece16& x, const StringPiece16& y) {
const int r = StringPiece16::wordmemcmp(
const int r = CharTraits<StringPiece16::value_type>::compare(
x.data(), y.data(), (x.size() < y.size() ? x.size() : y.size()));
return ((r < 0) || ((r == 0) && (x.size() < y.size())));
}
Expand Down
96 changes: 96 additions & 0 deletions base/strings/string_piece_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -706,4 +706,100 @@ TYPED_TEST(CommonStringPieceTest, CheckConstructors) {
ASSERT_EQ(empty, BasicStringPiece<TypeParam>(empty.begin(), empty.end()));
}

TEST(StringPieceTest, ConstexprCtor) {
{
constexpr StringPiece piece;
std::ignore = piece;
}

{
constexpr StringPiece piece("abc");
std::ignore = piece;
}

{
constexpr StringPiece piece("abc", 2);
std::ignore = piece;
}
}

TEST(StringPieceTest, ConstexprData) {
{
constexpr StringPiece piece;
static_assert(piece.data() == nullptr, "");
}

{
constexpr StringPiece piece("abc");
static_assert(piece.data()[0] == 'a', "");
static_assert(piece.data()[1] == 'b', "");
static_assert(piece.data()[2] == 'c', "");
}

{
constexpr StringPiece piece("def", 2);
static_assert(piece.data()[0] == 'd', "");
static_assert(piece.data()[1] == 'e', "");
}
}

TEST(StringPieceTest, ConstexprSize) {
{
constexpr StringPiece piece;
static_assert(piece.size() == 0, "");
}

{
constexpr StringPiece piece("abc");
static_assert(piece.size() == 3, "");
}

{
constexpr StringPiece piece("def", 2);
static_assert(piece.size() == 2, "");
}
}

TEST(StringPieceTest, Compare) {
constexpr StringPiece piece = "def";

static_assert(piece.compare("ab") == 1, "");
static_assert(piece.compare("abc") == 1, "");
static_assert(piece.compare("abcd") == 1, "");
static_assert(piece.compare("de") == 1, "");
static_assert(piece.compare("def") == 0, "");
static_assert(piece.compare("defg") == -1, "");
static_assert(piece.compare("gh") == -1, "");
static_assert(piece.compare("ghi") == -1, "");
static_assert(piece.compare("ghij") == -1, "");
}

TEST(StringPieceTest, StartsWith) {
constexpr StringPiece piece("abc");

static_assert(piece.starts_with(""), "");
static_assert(piece.starts_with("a"), "");
static_assert(piece.starts_with("ab"), "");
static_assert(piece.starts_with("abc"), "");

static_assert(!piece.starts_with("b"), "");
static_assert(!piece.starts_with("bc"), "");

static_assert(!piece.starts_with("abcd"), "");
}

TEST(StringPieceTest, EndsWith) {
constexpr StringPiece piece("abc");

static_assert(piece.ends_with(""), "");
static_assert(piece.ends_with("c"), "");
static_assert(piece.ends_with("bc"), "");
static_assert(piece.ends_with("abc"), "");

static_assert(!piece.ends_with("a"), "");
static_assert(!piece.ends_with("ab"), "");

static_assert(!piece.ends_with("abcd"), "");
}

} // namespace base
20 changes: 5 additions & 15 deletions components/viz/service/display/shader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,26 @@
#include <vector>

#include "base/logging.h"
#include "base/strings/char_traits.h"
#include "base/strings/stringprintf.h"
#include "components/viz/service/display/static_geometry_binding.h"
#include "gpu/command_buffer/client/gles2_interface.h"
#include "ui/gfx/color_transform.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/size.h"

constexpr bool ConstexprEqual(const char* a, const char* b, size_t length) {
for (size_t i = 0; i < length; i++) {
if (a[i] != b[i])
return false;
}
return true;
}

constexpr base::StringPiece StripLambda(base::StringPiece shader) {
// Must contain at least "[]() {}" and trailing null (included in size).
// TODO(jbroman): Simplify this once we're in a post-C++17 world, where
// starts_with and ends_with can easily be made constexpr.
DCHECK(shader.size() >= 7); // NOLINT
DCHECK(ConstexprEqual(shader.data(), "[]() {", 6));
// Must contain at least "[]() {}".
DCHECK(shader.starts_with("[]() {"));
DCHECK(shader.ends_with("}"));
shader.remove_prefix(6);
DCHECK(shader[shader.size() - 1] == '}'); // NOLINT
shader.remove_suffix(1);
return shader;
}

// Shaders are passed in with lambda syntax, which tricks clang-format into
// handling them correctly. StripLambda removes this.
#define SHADER0(Src) StripLambda(base::StringPiece(#Src, sizeof(#Src) - 1))
#define SHADER0(Src) StripLambda(#Src)

#define HDR(x) \
do { \
Expand Down
Loading

0 comments on commit 154c940

Please sign in to comment.