From f21268aa72a692750facc6ef681a9f782148751a Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sun, 8 Jul 2018 08:05:55 -0700 Subject: [PATCH] Revert "Optimize format string parsing" because of a bug in MSVC https://godbolt.org/g/rpiDgh This reverts commit f9e9bf02310e38e70bbe40508f0286792e68aa32. --- include/fmt/core.h | 16 ++++---------- include/fmt/format.h | 51 ++++++++++++++++++++------------------------ test/format-test.cc | 8 +++---- 3 files changed, 30 insertions(+), 45 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 3427e6c1..1303f559 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -212,14 +212,7 @@ FMT_CONSTEXPR typename std::make_unsigned::type to_unsigned(Int value) { return static_cast::type>(value); } -// A constexpr std::char_traits::length replacement for pre-C++17. -template -FMT_CONSTEXPR size_t length(const Char *s) { - const Char *start = s; - while (*s) ++s; - return s - start; } -} // namespace internal /** An implementation of ``std::basic_string_view`` for pre-C++17. It provides a @@ -262,8 +255,8 @@ class basic_string_view { the size with ``std::char_traits::length``. \endrst */ - FMT_CONSTEXPR basic_string_view(const Char *s) - : data_(s), size_(internal::length(s)) {} + basic_string_view(const Char *s) + : data_(s), size_(std::char_traits::length(s)) {} /** Constructs a string reference from a ``std::basic_string`` object. */ template @@ -275,7 +268,7 @@ class basic_string_view { : data_(s.data()), size_(s.size()) {} /** Returns a pointer to the string data. */ - FMT_CONSTEXPR const Char *data() const { return data_; } + const Char *data() const { return data_; } /** Returns the string size. */ FMT_CONSTEXPR size_t size() const { return size_; } @@ -1095,8 +1088,7 @@ class basic_format_args { format_arg do_get(size_type index) const { long long signed_types = static_cast(types_); if (signed_types < 0) { - unsigned long long num_args = - static_cast(-signed_types); + unsigned long long num_args = static_cast(-signed_types); return index < num_args ? args_[index] : format_arg(); } format_arg arg; diff --git a/include/fmt/format.h b/include/fmt/format.h index 9e703aab..8b6fadd3 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -737,7 +737,7 @@ class null_terminating_iterator { FMT_CONSTEXPR explicit null_terminating_iterator(const Range &r) : ptr_(r.begin()), end_(r.end()) {} - FMT_CONSTEXPR null_terminating_iterator &operator=(const Char *ptr) { + null_terminating_iterator &operator=(const Char *ptr) { assert(ptr <= end_); ptr_ = ptr; return *this; @@ -2138,28 +2138,25 @@ struct id_adapter { Handler &handler; }; -template -FMT_CONSTEXPR void parse_format_string( - basic_string_view format_str, Handler &&handler) { - auto begin = format_str.data(); - auto end = begin + format_str.size(); - auto p = begin; - while (p != end) { - Char ch = *p++; +template +FMT_CONSTEXPR void parse_format_string(Iterator it, Handler &&handler) { + typedef typename std::iterator_traits::value_type char_type; + auto start = it; + while (*it) { + char_type ch = *it++; if (ch != '{' && ch != '}') continue; - if (p != end && *p == ch) { - handler.on_text(begin, p); - begin = ++p; + if (*it == ch) { + handler.on_text(start, it); + start = ++it; continue; } if (ch == '}') { handler.on_error("unmatched '}' in format string"); return; } - handler.on_text(begin, p - 1); + handler.on_text(start, it - 1); - internal::null_terminating_iterator it(p, end); - it = parse_arg_id(it, id_adapter(handler)); + it = parse_arg_id(it, id_adapter(handler)); if (*it == '}') { handler.on_replacement_field(it); } else if (*it == ':') { @@ -2173,11 +2170,10 @@ FMT_CONSTEXPR void parse_format_string( handler.on_error("missing '}' in format string"); return; } - p = pointer_from(it); - begin = ++p; + start = ++it; } - handler.on_text(begin, p); + handler.on_text(start, it); } template @@ -2196,8 +2192,6 @@ class format_string_checker { : arg_id_(-1), context_(format_str, eh), parse_funcs_{&parse_format_specs...} {} - typedef internal::null_terminating_iterator iterator; - FMT_CONSTEXPR void on_text(const Char *, const Char *) {} FMT_CONSTEXPR void on_arg_id() { @@ -2211,12 +2205,12 @@ class format_string_checker { } FMT_CONSTEXPR void on_arg_id(basic_string_view) {} - FMT_CONSTEXPR void on_replacement_field(iterator) {} + FMT_CONSTEXPR void on_replacement_field(const Char *) {} - FMT_CONSTEXPR const Char *on_format_specs(iterator it) { - auto p = pointer_from(it); - context_.advance_to(p); - return to_unsigned(arg_id_) < NUM_ARGS ? parse_funcs_[arg_id_](context_) : p; + FMT_CONSTEXPR const Char *on_format_specs(const Char *s) { + context_.advance_to(s); + return to_unsigned(arg_id_) < NUM_ARGS ? + parse_funcs_[arg_id_](context_) : s; } FMT_CONSTEXPR void on_error(const char *message) { @@ -2244,7 +2238,7 @@ template FMT_CONSTEXPR bool check_format_string( basic_string_view s, ErrorHandler eh = ErrorHandler()) { format_string_checker checker(s, eh); - parse_format_string(s, checker); + parse_format_string(s.begin(), checker); return true; } @@ -3303,7 +3297,7 @@ struct format_handler : internal::error_handler { basic_format_args format_args) : context(r.begin(), str, format_args) {} - void on_text(const Char *begin, const Char *end) { + void on_text(iterator begin, iterator end) { auto size = internal::to_unsigned(end - begin); auto out = context.out(); auto &&it = internal::reserve(out, size); @@ -3354,8 +3348,9 @@ template typename Context::iterator vformat_to(typename ArgFormatter::range out, basic_string_view format_str, basic_format_args args) { + typedef internal::null_terminating_iterator iterator; format_handler h(out, format_str, args); - parse_format_string(format_str, h); + parse_format_string(iterator(format_str.begin(), format_str.end()), h); return h.context.out(); } diff --git a/test/format-test.cc b/test/format-test.cc index 548b215f..3d692589 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -1760,18 +1760,16 @@ struct test_format_string_handler { template FMT_CONSTEXPR void on_arg_id(T) {} - template - FMT_CONSTEXPR void on_replacement_field(Iterator) {} + FMT_CONSTEXPR void on_replacement_field(const char *) {} - template - FMT_CONSTEXPR Iterator on_format_specs(Iterator it) { return it; } + FMT_CONSTEXPR const char *on_format_specs(const char *s) { return s; } FMT_CONSTEXPR void on_error(const char *) { error = true; } bool error = false; }; -FMT_CONSTEXPR bool parse_string(fmt::string_view s) { +FMT_CONSTEXPR bool parse_string(const char *s) { test_format_string_handler h; fmt::internal::parse_format_string(s, h); return !h.error;