From 5a1127b726879e21e5a9613fc620d08413d39054 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sun, 14 Mar 2021 09:08:08 -0700 Subject: [PATCH 1/4] Don't wrap named arg in cref and clarify docs --- include/fmt/args.h | 24 ++++++++---------------- include/fmt/core.h | 5 +++-- test/core-test.cc | 22 ++++------------------ 3 files changed, 15 insertions(+), 36 deletions(-) diff --git a/include/fmt/args.h b/include/fmt/args.h index 5891ea25ff23..b55fbdb7ba94 100644 --- a/include/fmt/args.h +++ b/include/fmt/args.h @@ -168,29 +168,21 @@ class dynamic_format_arg_store /** \rst Adds a reference to the argument into the dynamic store for later passing to - a formatting function. Supports named arguments wrapped in - ``std::reference_wrapper`` via ``std::ref()``/``std::cref()``. + a formatting function. **Example**:: fmt::dynamic_format_arg_store store; - char str[] = "1234567890"; - store.push_back(std::cref(str)); - int a1_val{42}; - auto a1 = fmt::arg("a1_", a1_val); - store.push_back(std::cref(a1)); - - // Changing str affects the output but only for string and custom types. - str[0] = 'X'; - - std::string result = fmt::vformat("{} and {a1_}"); - assert(result == "X234567890 and 42"); + char band[] = "Rolling Stones"; + store.push_back(std::cref(band)); + band[9] = 'c'; // Changing str affects the output. + std::string result = fmt::vformat("{}", store); + // result == "Rolling Scones" \endrst */ template void push_back(std::reference_wrapper arg) { static_assert( - detail::is_named_arg::type>::value || - need_copy::value, + need_copy::value, "objects of built-in types and string views are always copied"); emplace_arg(arg.get()); } @@ -198,7 +190,7 @@ class dynamic_format_arg_store /** Adds named argument into the dynamic store for later passing to a formatting function. ``std::reference_wrapper`` is supported to avoid copying of the - argument. + argument. The name is always stored by reference. */ template void push_back(const detail::named_arg& arg) { diff --git a/include/fmt/core.h b/include/fmt/core.h index 68cc7964fdd6..d0e06e269b0f 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1619,8 +1619,9 @@ inline auto make_args_checked(const S& format_str, /** \rst - Returns a named argument to be used in a formatting function. It should only - be used in a call to a formatting function. + Returns a named argument to be used in a formatting function. + It should only be used in a call to a formatting function or + `dynamic_format_arg_store::push_back`. **Example**:: diff --git a/test/core-test.cc b/test/core-test.cc index 5da58a60f07e..b24d66355bae 100644 --- a/test/core-test.cc +++ b/test/core-test.cc @@ -469,24 +469,10 @@ TEST(FormatDynArgsTest, NamedStrings) { TEST(FormatDynArgsTest, NamedArgByRef) { fmt::dynamic_format_arg_store store; - - // Note: fmt::arg() constructs an object which holds a reference - // to its value. It's not an aggregate, so it doesn't extend the - // reference lifetime. As a result, it's a very bad idea passing temporary - // as a named argument value. Only GCC with optimization level >0 - // complains about this. - // - // A real life usecase is when you have both name and value alive - // guarantee their lifetime and thus don't want them to be copied into - // storages. - int a1_val{42}; - auto a1 = fmt::arg("a1_", a1_val); - store.push_back("abc"); - store.push_back(1.5f); - store.push_back(std::cref(a1)); - - std::string result = fmt::vformat("{a1_} and {} and {} and {}", store); - EXPECT_EQ("42 and abc and 1.5 and 42", result); + char band[] = "Rolling Stones"; + store.push_back(fmt::arg("band", std::cref(band))); + band[9] = 'c'; // Changing str affects the output. + EXPECT_EQ(fmt::vformat("{band}", store), "Rolling Scones"); } TEST(FormatDynArgsTest, NamedCustomFormat) { From 6151d0dc1e7c0deb40a3a5d9e1ac3f506830555c Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sun, 14 Mar 2021 09:26:18 -0700 Subject: [PATCH 2/4] Fix the comment --- test/core-test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core-test.cc b/test/core-test.cc index b24d66355bae..6c39551591dd 100644 --- a/test/core-test.cc +++ b/test/core-test.cc @@ -471,7 +471,7 @@ TEST(FormatDynArgsTest, NamedArgByRef) { fmt::dynamic_format_arg_store store; char band[] = "Rolling Stones"; store.push_back(fmt::arg("band", std::cref(band))); - band[9] = 'c'; // Changing str affects the output. + band[9] = 'c'; // Changing band affects the output. EXPECT_EQ(fmt::vformat("{band}", store), "Rolling Scones"); } From 8308f52c2aaddbe08f9ed20e4ce9f46f6bc29e3f Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Mon, 15 Mar 2021 07:10:28 -0700 Subject: [PATCH 3/4] Fix dynamic_format_arg_store::push_back comment --- include/fmt/args.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fmt/args.h b/include/fmt/args.h index b55fbdb7ba94..0fe759325d11 100644 --- a/include/fmt/args.h +++ b/include/fmt/args.h @@ -190,7 +190,7 @@ class dynamic_format_arg_store /** Adds named argument into the dynamic store for later passing to a formatting function. ``std::reference_wrapper`` is supported to avoid copying of the - argument. The name is always stored by reference. + argument. The name is always copied into the store. */ template void push_back(const detail::named_arg& arg) { From d0bded5988198abbaf3974bdedd30a6cb48c0562 Mon Sep 17 00:00:00 2001 From: Brainy0207 <80522992+Brainy0207@users.noreply.github.com> Date: Mon, 15 Mar 2021 16:17:28 +0100 Subject: [PATCH 4/4] Fix MSVC /clr builds (#2179) --- include/fmt/format.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/include/fmt/format.h b/include/fmt/format.h index 06f85a7ce31a..1a76ebd4471f 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -204,17 +204,18 @@ FMT_END_NAMESPACE // __builtin_clz and __builtin_clzll, so only define FMT_BUILTIN_CLZ using the // MSVC intrinsics if the clz and clzll builtins are not available. #if FMT_MSC_VER && !defined(FMT_BUILTIN_CLZLL) && \ - !defined(FMT_BUILTIN_CTZLL) && !defined(_MANAGED) + !defined(FMT_BUILTIN_CTZLL) FMT_BEGIN_NAMESPACE namespace detail { // Avoid Clang with Microsoft CodeGen's -Wunknown-pragmas warning. -# ifndef __clang__ +# if !defined(__clang__) +# pragma managed(push, off) # pragma intrinsic(_BitScanForward) # pragma intrinsic(_BitScanReverse) -# endif -# if defined(_WIN64) && !defined(__clang__) -# pragma intrinsic(_BitScanForward64) -# pragma intrinsic(_BitScanReverse64) +# if defined(_WIN64) +# pragma intrinsic(_BitScanForward64) +# pragma intrinsic(_BitScanReverse64) +# endif # endif inline int clz(uint32_t x) { @@ -270,6 +271,9 @@ inline int ctzll(uint64_t x) { return static_cast(r); } # define FMT_BUILTIN_CTZLL(n) detail::ctzll(n) +# if !defined(__clang__) +# pragma managed(pop) +# endif } // namespace detail FMT_END_NAMESPACE #endif