Revert "Optimize format string parsing" because of a bug in MSVC

https://godbolt.org/g/rpiDgh

This reverts commit f9e9bf0231.

include/fmt/core.h

@@ -212,14 +212,7 @@ FMT_CONSTEXPR typename std::make_unsigned<Int>::type to_unsigned(Int value) {
   return static_cast<typename std::make_unsigned<Int>::type>(value);
 }
 
-// A constexpr std::char_traits::length replacement for pre-C++17.
-template <typename Char>
-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<Char>::length``.
     \endrst
    */
-  FMT_CONSTEXPR basic_string_view(const Char *s)
+  basic_string_view(const Char *s)
-    : data_(s), size_(internal::length(s)) {}
+    : data_(s), size_(std::char_traits<Char>::length(s)) {}
 
   /** Constructs a string reference from a ``std::basic_string`` object. */
   template <typename Alloc>
@@ -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<long long>(types_);
     if (signed_types < 0) {
-      unsigned long long num_args =
+      unsigned long long num_args = static_cast<unsigned long long>(-signed_types);
-          static_cast<unsigned long long>(-signed_types);
       return index < num_args ? args_[index] : format_arg();
     }
     format_arg arg;

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 <typename Char, typename Handler>
+template <typename Iterator, typename Handler>
-FMT_CONSTEXPR void parse_format_string(
+FMT_CONSTEXPR void parse_format_string(Iterator it, Handler &&handler) {
-        basic_string_view<Char> format_str, Handler &&handler) {
+  typedef typename std::iterator_traits<Iterator>::value_type char_type;
-  auto begin = format_str.data();
+  auto start = it;
-  auto end = begin + format_str.size();
+  while (*it) {
-  auto p = begin;
+    char_type ch = *it++;
-  while (p != end) {
-    Char ch = *p++;
     if (ch != '{' && ch != '}') continue;
-    if (p != end && *p == ch) {
+    if (*it == ch) {
-      handler.on_text(begin, p);
+      handler.on_text(start, it);
-      begin = ++p;
+      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<Char> it(p, end);
+    it = parse_arg_id(it, id_adapter<Handler, char_type>(handler));
-    it = parse_arg_id(it, id_adapter<Handler, Char>(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 <typename T, typename ParseContext>
@@ -2196,8 +2192,6 @@ class format_string_checker {
     : arg_id_(-1), context_(format_str, eh),
       parse_funcs_{&parse_format_specs<Args, parse_context_type>...} {}
 
-  typedef internal::null_terminating_iterator<Char> 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<Char>) {}
 
-  FMT_CONSTEXPR void on_replacement_field(iterator) {}
+  FMT_CONSTEXPR void on_replacement_field(const Char *) {}
 
-  FMT_CONSTEXPR const Char *on_format_specs(iterator it) {
+  FMT_CONSTEXPR const Char *on_format_specs(const Char *s) {
-    auto p = pointer_from(it);
+    context_.advance_to(s);
-    context_.advance_to(p);
+    return to_unsigned(arg_id_) < NUM_ARGS ?
-    return to_unsigned(arg_id_) < NUM_ARGS ? parse_funcs_[arg_id_](context_) : p;
+          parse_funcs_[arg_id_](context_) : s;
   }
 
   FMT_CONSTEXPR void on_error(const char *message) {
@@ -2244,7 +2238,7 @@ template <typename Char, typename ErrorHandler, typename... Args>
 FMT_CONSTEXPR bool check_format_string(
     basic_string_view<Char> s, ErrorHandler eh = ErrorHandler()) {
   format_string_checker<Char, ErrorHandler, Args...> 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<Context> 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 ArgFormatter, typename Char, typename Context>
 typename Context::iterator vformat_to(typename ArgFormatter::range out,
                                       basic_string_view<Char> format_str,
                                       basic_format_args<Context> args) {
+  typedef internal::null_terminating_iterator<Char> iterator;
   format_handler<ArgFormatter, Char, Context> 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();
 }
 

test/format-test.cc

@@ -1760,18 +1760,16 @@ struct test_format_string_handler {
   template <typename T>
   FMT_CONSTEXPR void on_arg_id(T) {}
 
-  template <typename Iterator>
+  FMT_CONSTEXPR void on_replacement_field(const char *) {}
-  FMT_CONSTEXPR void on_replacement_field(Iterator) {}
 
-  template <typename Iterator>
+  FMT_CONSTEXPR const char *on_format_specs(const char *s) { return s; }
-  FMT_CONSTEXPR Iterator on_format_specs(Iterator it) { return it; }
 
   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;