From 167ec541407c7207b5a527a537cb057c85afb756 Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 22 Feb 2025 09:39:29 -0700 Subject: [PATCH 01/10] Report error on duplicate named arg names #4282 --- include/fmt/base.h | 10 ++++++++++ test/compile-error-test/CMakeLists.txt | 8 ++++++++ test/format-test.cc | 2 ++ 3 files changed, 20 insertions(+) diff --git a/include/fmt/base.h b/include/fmt/base.h index a37858a843c5..7be57f09ce2a 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1071,6 +1071,11 @@ void init_named_arg(named_arg_info*, int& arg_index, int&, const T&) { template ::value)> void init_named_arg(named_arg_info* named_args, int& arg_index, int& named_arg_index, const T& arg) { + for (auto i = 0; i < named_arg_index; ++i) { + if (basic_string_view(named_args[i].name) == basic_string_view(arg.name)) { + report_error("duplicate named args found"); + } + } named_args[named_arg_index++] = {arg.name, arg_index++}; } @@ -1084,6 +1089,11 @@ template ::value)> FMT_CONSTEXPR void init_static_named_arg(named_arg_info* named_args, int& arg_index, int& named_arg_index) { + for (auto i = 0; i < named_arg_index; ++i) { + if (basic_string_view(named_args[i].name) == basic_string_view(T::name)) { + report_error("duplicate named args found"); + } + } named_args[named_arg_index++] = {T::name, arg_index++}; } diff --git a/test/compile-error-test/CMakeLists.txt b/test/compile-error-test/CMakeLists.txt index a86996e21f95..b2586a76af27 100644 --- a/test/compile-error-test/CMakeLists.txt +++ b/test/compile-error-test/CMakeLists.txt @@ -209,6 +209,14 @@ if (CMAKE_CXX_STANDARD GREATER_EQUAL 20) #error #endif " ERROR) + expect_compile(format-string-duplicate-name-error " + #if defined(FMT_HAS_CONSTEVAL) && FMT_USE_NONTYPE_TEMPLATE_ARGS + using namespace fmt::literals; + fmt::print(\"{bar}\", \"bar\"_a=42, \"bar\"_a=43); + #else + #error + #endif + " ERROR) endif () # Run all tests diff --git a/test/format-test.cc b/test/format-test.cc index c16f895bed29..ce5db19ec39f 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -582,6 +582,8 @@ TEST(format_test, named_arg) { EXPECT_EQ("1/a/A", fmt::format("{_1}/{a_}/{A_}", fmt::arg("a_", 'a'), fmt::arg("A_", "A"), fmt::arg("_1", 1))); EXPECT_EQ(fmt::format("{0:{width}}", -42, fmt::arg("width", 4)), " -42"); + EXPECT_THROW_MSG(fmt::format("{enum}", fmt::arg("enum", 1), fmt::arg("enum", 10)), + format_error, "duplicate named args found"); EXPECT_EQ("st", fmt::format("{0:.{precision}}", "str", fmt::arg("precision", 2))); EXPECT_EQ(fmt::format("{} {two}", 1, fmt::arg("two", 2)), "1 2"); From 233d8d9e859f27f6513bcda4bd1e56153f4e01be Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Wed, 26 Feb 2025 08:16:34 -0700 Subject: [PATCH 02/10] Fix unused return warning --- test/format-test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/format-test.cc b/test/format-test.cc index ce5db19ec39f..e2cca39c5434 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -582,8 +582,6 @@ TEST(format_test, named_arg) { EXPECT_EQ("1/a/A", fmt::format("{_1}/{a_}/{A_}", fmt::arg("a_", 'a'), fmt::arg("A_", "A"), fmt::arg("_1", 1))); EXPECT_EQ(fmt::format("{0:{width}}", -42, fmt::arg("width", 4)), " -42"); - EXPECT_THROW_MSG(fmt::format("{enum}", fmt::arg("enum", 1), fmt::arg("enum", 10)), - format_error, "duplicate named args found"); EXPECT_EQ("st", fmt::format("{0:.{precision}}", "str", fmt::arg("precision", 2))); EXPECT_EQ(fmt::format("{} {two}", 1, fmt::arg("two", 2)), "1 2"); @@ -601,6 +599,8 @@ TEST(format_test, named_arg) { EXPECT_THROW_MSG((void)fmt::format(runtime("{a} {}"), fmt::arg("a", 2), 42), format_error, "cannot switch from manual to automatic argument indexing"); + EXPECT_THROW_MSG((void)fmt::format("{enum}", fmt::arg("enum", 1), + fmt::arg("enum", 10)), format_error, "duplicate named args found"); } TEST(format_test, auto_arg_index) { From 3d9a3e669e0bbf57b93d16749868ad0146e2e347 Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Wed, 26 Feb 2025 08:37:05 -0700 Subject: [PATCH 03/10] Fix formatting --- include/fmt/base.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/fmt/base.h b/include/fmt/base.h index 7be57f09ce2a..e0a8582e77c8 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1072,7 +1072,8 @@ template ::value)> void init_named_arg(named_arg_info* named_args, int& arg_index, int& named_arg_index, const T& arg) { for (auto i = 0; i < named_arg_index; ++i) { - if (basic_string_view(named_args[i].name) == basic_string_view(arg.name)) { + if (basic_string_view(named_args[i].name) == + basic_string_view(arg.name)) { report_error("duplicate named args found"); } } @@ -1090,7 +1091,8 @@ template * named_args, int& arg_index, int& named_arg_index) { for (auto i = 0; i < named_arg_index; ++i) { - if (basic_string_view(named_args[i].name) == basic_string_view(T::name)) { + if (basic_string_view(named_args[i].name) == + basic_string_view(T::name)) { report_error("duplicate named args found"); } } From 93b1425019d3212360648287466b9ddc1d788776 Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 1 Mar 2025 07:57:42 -0700 Subject: [PATCH 04/10] Refactor check function, optimize check function, remove expensive test --- include/fmt/base.h | 27 ++++++++++++++------------ test/compile-error-test/CMakeLists.txt | 8 -------- test/format-test.cc | 4 ++-- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/include/fmt/base.h b/include/fmt/base.h index e0a8582e77c8..df8136d0da24 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1064,19 +1064,27 @@ template struct named_arg_info { int id; }; +template +FMT_CONSTEXPR void check_for_duplicate( + const named_arg_info* const named_args, const int named_arg_index, + const Char* const arg_name) { + const basic_string_view arg_name_view(arg_name); + for (auto i = 0; i < named_arg_index; ++i) { + if (basic_string_view(named_args[i].name) == arg_name_view) { + report_error("duplicate named args found"); + } + } +} + template ::value)> void init_named_arg(named_arg_info*, int& arg_index, int&, const T&) { ++arg_index; } + template ::value)> void init_named_arg(named_arg_info* named_args, int& arg_index, int& named_arg_index, const T& arg) { - for (auto i = 0; i < named_arg_index; ++i) { - if (basic_string_view(named_args[i].name) == - basic_string_view(arg.name)) { - report_error("duplicate named args found"); - } - } + check_for_duplicate(named_args, named_arg_index, arg.name); named_args[named_arg_index++] = {arg.name, arg_index++}; } @@ -1090,12 +1098,7 @@ template ::value)> FMT_CONSTEXPR void init_static_named_arg(named_arg_info* named_args, int& arg_index, int& named_arg_index) { - for (auto i = 0; i < named_arg_index; ++i) { - if (basic_string_view(named_args[i].name) == - basic_string_view(T::name)) { - report_error("duplicate named args found"); - } - } + check_for_duplicate(named_args, named_arg_index, T::name); named_args[named_arg_index++] = {T::name, arg_index++}; } diff --git a/test/compile-error-test/CMakeLists.txt b/test/compile-error-test/CMakeLists.txt index b2586a76af27..a86996e21f95 100644 --- a/test/compile-error-test/CMakeLists.txt +++ b/test/compile-error-test/CMakeLists.txt @@ -209,14 +209,6 @@ if (CMAKE_CXX_STANDARD GREATER_EQUAL 20) #error #endif " ERROR) - expect_compile(format-string-duplicate-name-error " - #if defined(FMT_HAS_CONSTEVAL) && FMT_USE_NONTYPE_TEMPLATE_ARGS - using namespace fmt::literals; - fmt::print(\"{bar}\", \"bar\"_a=42, \"bar\"_a=43); - #else - #error - #endif - " ERROR) endif () # Run all tests diff --git a/test/format-test.cc b/test/format-test.cc index e2cca39c5434..e2c595c9a8af 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -599,8 +599,8 @@ TEST(format_test, named_arg) { EXPECT_THROW_MSG((void)fmt::format(runtime("{a} {}"), fmt::arg("a", 2), 42), format_error, "cannot switch from manual to automatic argument indexing"); - EXPECT_THROW_MSG((void)fmt::format("{enum}", fmt::arg("enum", 1), - fmt::arg("enum", 10)), format_error, "duplicate named args found"); + EXPECT_THROW_MSG((void)fmt::format("{a}", fmt::arg("a", 1), + fmt::arg("a", 10)), format_error, "duplicate named args found"); } TEST(format_test, auto_arg_index) { From 36a6908d1a5b31544e54c9f56b2e21d18f94d065 Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 1 Mar 2025 08:03:56 -0700 Subject: [PATCH 05/10] Fix missing CTAD in C++11 --- include/fmt/base.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fmt/base.h b/include/fmt/base.h index df8136d0da24..ce0a97aad017 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1068,7 +1068,7 @@ template FMT_CONSTEXPR void check_for_duplicate( const named_arg_info* const named_args, const int named_arg_index, const Char* const arg_name) { - const basic_string_view arg_name_view(arg_name); + const basic_string_view arg_name_view(arg_name); for (auto i = 0; i < named_arg_index; ++i) { if (basic_string_view(named_args[i].name) == arg_name_view) { report_error("duplicate named args found"); From b6be6feb16c4617f6acf5cd73e7a3a14875340b6 Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 1 Mar 2025 09:08:18 -0700 Subject: [PATCH 06/10] Fix GCC13 build and enable GCC10 C++11 build --- include/fmt/base.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/fmt/base.h b/include/fmt/base.h index ce0a97aad017..b401386c079e 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1060,6 +1060,11 @@ template constexpr auto count_static_named_args() -> int { } template struct named_arg_info { + FMT_CONSTEXPR named_arg_info() : name(nullptr), id(0) {} + FMT_CONSTEXPR named_arg_info(const Char* a_name, const int an_id) + : name(a_name), + id(an_id) { + } const Char* name; int id; }; From 1c156a3bb9e2f1564a5db2691aacb9f16e668f5b Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 1 Mar 2025 09:19:29 -0700 Subject: [PATCH 07/10] Fix formatting --- include/fmt/base.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/fmt/base.h b/include/fmt/base.h index b401386c079e..02015becfdc2 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1062,9 +1062,7 @@ template constexpr auto count_static_named_args() -> int { template struct named_arg_info { FMT_CONSTEXPR named_arg_info() : name(nullptr), id(0) {} FMT_CONSTEXPR named_arg_info(const Char* a_name, const int an_id) - : name(a_name), - id(an_id) { - } + : name(a_name), id(an_id) {} const Char* name; int id; }; From 980014780902c2d29241b04a259c86ec2cbe171d Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 1 Mar 2025 09:24:31 -0700 Subject: [PATCH 08/10] Fix formatting, again --- include/fmt/base.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fmt/base.h b/include/fmt/base.h index 02015becfdc2..2e93436de540 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1062,7 +1062,7 @@ template constexpr auto count_static_named_args() -> int { template struct named_arg_info { FMT_CONSTEXPR named_arg_info() : name(nullptr), id(0) {} FMT_CONSTEXPR named_arg_info(const Char* a_name, const int an_id) - : name(a_name), id(an_id) {} + : name(a_name), id(an_id) {} const Char* name; int id; }; From 7cf7957bc0245d06985a9ccbe4efcd338adfba27 Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 1 Mar 2025 20:35:58 -0700 Subject: [PATCH 09/10] Remove init methods, simplify const usage, simplify check function, return early based on named_arg_index --- include/fmt/base.h | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/include/fmt/base.h b/include/fmt/base.h index 2e93436de540..3d86158fb136 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1060,20 +1060,18 @@ template constexpr auto count_static_named_args() -> int { } template struct named_arg_info { - FMT_CONSTEXPR named_arg_info() : name(nullptr), id(0) {} - FMT_CONSTEXPR named_arg_info(const Char* a_name, const int an_id) - : name(a_name), id(an_id) {} const Char* name; int id; }; template FMT_CONSTEXPR void check_for_duplicate( - const named_arg_info* const named_args, const int named_arg_index, - const Char* const arg_name) { - const basic_string_view arg_name_view(arg_name); + named_arg_info* named_args, int named_arg_index, + basic_string_view arg_name) { + if (named_arg_index <= 0) return; + for (auto i = 0; i < named_arg_index; ++i) { - if (basic_string_view(named_args[i].name) == arg_name_view) { + if (basic_string_view(named_args[i].name) == arg_name) { report_error("duplicate named args found"); } } @@ -1087,7 +1085,7 @@ void init_named_arg(named_arg_info*, int& arg_index, int&, const T&) { template ::value)> void init_named_arg(named_arg_info* named_args, int& arg_index, int& named_arg_index, const T& arg) { - check_for_duplicate(named_args, named_arg_index, arg.name); + check_for_duplicate(named_args, named_arg_index, arg.name); named_args[named_arg_index++] = {arg.name, arg_index++}; } @@ -1101,7 +1099,7 @@ template ::value)> FMT_CONSTEXPR void init_static_named_arg(named_arg_info* named_args, int& arg_index, int& named_arg_index) { - check_for_duplicate(named_args, named_arg_index, T::name); + check_for_duplicate(named_args, named_arg_index, T::name); named_args[named_arg_index++] = {T::name, arg_index++}; } From e3bfecd8e5c34db2e4ca37159e68e9619e73bc03 Mon Sep 17 00:00:00 2001 From: Dean Glazeski Date: Sat, 1 Mar 2025 20:43:26 -0700 Subject: [PATCH 10/10] Run clang-format --- include/fmt/base.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/fmt/base.h b/include/fmt/base.h index 3d86158fb136..dc36728bc777 100644 --- a/include/fmt/base.h +++ b/include/fmt/base.h @@ -1065,9 +1065,9 @@ template struct named_arg_info { }; template -FMT_CONSTEXPR void check_for_duplicate( - named_arg_info* named_args, int named_arg_index, - basic_string_view arg_name) { +FMT_CONSTEXPR void check_for_duplicate(named_arg_info* named_args, + int named_arg_index, + basic_string_view arg_name) { if (named_arg_index <= 0) return; for (auto i = 0; i < named_arg_index; ++i) {