From c970d4bc21134f893e933726f5fdaad464db35b3 Mon Sep 17 00:00:00 2001 From: Marek Kurdej Date: Fri, 30 Aug 2019 15:38:37 +0200 Subject: [PATCH] Add compile-time check for too many arguments: * Check only for automatic argument indexing. * Tests not included as I am not sure how compile failures should be tested (I haven't seen any death test in the repo). All the following lines fail to compile after applying this patch: ``` fmt::print(FMT_STRING(""), 1); // too many fmt::print(FMT_STRING("too many\n"), 1); fmt::print(FMT_STRING("too many {}\n"), 1, 2); fmt::print(FMT_STRING("too many {}\n"), 1, 2, 3); fmt::print(FMT_STRING("too few {}-{}-{}bal\n"), 1, 2); ``` Tested on GCC and clang (https://wandbox.org/permlink/f7Pf9ERBskTWfFQx). --- include/fmt/compile.h | 5 +++++ include/fmt/format.h | 22 +++++++++++++++++++++- test/format-test.cc | 9 +++++---- test/scan.h | 2 ++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/include/fmt/compile.h b/include/fmt/compile.h index 082b0762..01abb701 100644 --- a/include/fmt/compile.h +++ b/include/fmt/compile.h @@ -9,6 +9,7 @@ #define FMT_COMPILE_H_ #include + #include "format.h" FMT_BEGIN_NAMESPACE @@ -109,6 +110,8 @@ class format_preparation_handler : public internal::error_handler { parts_.push_back(part(string_view_metadata(offset, size))); } + FMT_CONSTEXPR void on_text_end() {} + FMT_CONSTEXPR void on_arg_id() { parts_.push_back(part(parse_context_.next_arg_id())); } @@ -267,6 +270,8 @@ template struct part_counter { if (begin != end) ++num_parts; } + FMT_CONSTEXPR void on_text_end() {} + FMT_CONSTEXPR void on_arg_id() { ++num_parts; } FMT_CONSTEXPR void on_arg_id(unsigned) { ++num_parts; } FMT_CONSTEXPR void on_arg_id(basic_string_view) { ++num_parts; } diff --git a/include/fmt/format.h b/include/fmt/format.h index 08cf38dd..e1332bd2 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -2461,8 +2461,10 @@ FMT_CONSTEXPR void parse_format_string(basic_string_view format_str, // Doing two passes with memchr (one for '{' and another for '}') is up to // 2.5x faster than the naive one-pass implementation on big format strings. const Char* p = begin; - if (*begin != '{' && !find(begin, end, '{', p)) + if (*begin != '{' && !find(begin, end, '{', p)) { + handler.on_text_end(); return write(begin, end); + } write(begin, p); ++p; if (p == end) return handler.on_error("invalid format string"); @@ -2486,6 +2488,7 @@ FMT_CONSTEXPR void parse_format_string(basic_string_view format_str, } begin = p + 1; } + handler.on_text_end(); // Handle empty format strings. } template @@ -2510,17 +2513,31 @@ class format_string_checker { explicit FMT_CONSTEXPR format_string_checker( basic_string_view format_str, ErrorHandler eh) : arg_id_((std::numeric_limits::max)()), + max_arg_id_((std::numeric_limits::min)()), context_(format_str, eh), parse_funcs_{&parse_format_specs...} {} FMT_CONSTEXPR void on_text(const Char*, const Char*) {} + FMT_CONSTEXPR void on_text_end() { + // Do not check when using manual indexing. + if (max_arg_id_ == (std::numeric_limits::max)()) return; + + if (((arg_id_ == (std::numeric_limits::max)()) && + (num_args > 0)) || // No argument used. + (num_args > max_arg_id_ + 1)) // More arguments given than used. + return on_error("too many arguments"); + } + FMT_CONSTEXPR void on_arg_id() { arg_id_ = context_.next_arg_id(); + max_arg_id_ = ((arg_id_ > max_arg_id_) ? arg_id_ : max_arg_id_); check_arg_id(); } FMT_CONSTEXPR void on_arg_id(int id) { arg_id_ = id; + // Do not check for too many arguments when using manual argument indexing. + max_arg_id_ = (std::numeric_limits::max)(); context_.check_arg_id(id); check_arg_id(); } @@ -2551,6 +2568,7 @@ class format_string_checker { using parse_func = const Char* (*)(parse_context_type&); unsigned arg_id_; + unsigned max_arg_id_; parse_context_type context_; parse_func parse_funcs_[num_args > 0 ? num_args : 1]; }; @@ -3164,6 +3182,8 @@ struct format_handler : internal::error_handler { context.advance_to(out); } + void on_text_end() {} + void get_arg(int id) { arg = internal::get_arg(context, id); } void on_arg_id() { get_arg(parse_context.next_arg_id()); } diff --git a/test/format-test.cc b/test/format-test.cc index ffa0a9b2..e01efed9 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -6,6 +6,7 @@ // For the license information refer to format.h. #include + #include #include #include @@ -34,12 +35,12 @@ using std::size_t; using fmt::basic_memory_buffer; -using fmt::internal::basic_writer; using fmt::format; using fmt::format_error; using fmt::memory_buffer; using fmt::string_view; using fmt::wmemory_buffer; +using fmt::internal::basic_writer; using testing::Return; using testing::StrictMock; @@ -673,9 +674,7 @@ TEST(WriterTest, WriteString) { CHECK_WRITE_WCHAR("abc"); } -TEST(WriterTest, WriteWideString) { - CHECK_WRITE_WCHAR(L"abc"); -} +TEST(WriterTest, WriteWideString) { CHECK_WRITE_WCHAR(L"abc"); } TEST(FormatToTest, FormatWithoutArgs) { std::string s; @@ -2295,6 +2294,8 @@ TEST(FormatTest, ConstexprSpecsChecker) { struct test_format_string_handler { FMT_CONSTEXPR void on_text(const char*, const char*) {} + FMT_CONSTEXPR void on_text_end() {} + FMT_CONSTEXPR void on_arg_id() {} template FMT_CONSTEXPR void on_arg_id(T) {} diff --git a/test/scan.h b/test/scan.h index ace84f77..52e706fd 100644 --- a/test/scan.h +++ b/test/scan.h @@ -168,6 +168,8 @@ struct scan_handler : error_handler { scan_ctx_.advance_to(it + size); } + void on_text_end() {} + void on_arg_id() { on_arg_id(next_arg_id_++); } void on_arg_id(int id) { if (id >= args_.size) on_error("argument index out of range");