Format string checking incorrectly assumes repeated evaluation of string literal will produce the same pointer #4177
Comments
|
Applied the patch (with a small tweak) in cacc310. Thanks! |
|
@vitaut FMT_CONSTEXPR auto sv = string_view(S());? Instead of FMT_CONSTEXPR auto sv = string_view(s);?
|
|
I don't think it matters much but since the object is already available, I don't see why we need to construct another one. |
|
Adding a regression test: 1c5883b. |
Nevermind, I misinterpreted which is the current version of the code. You are right, we could use |
|
I did some local test by rolling our fmtlib to ToT and building our project with ToT clang but still getting errors: and |
|
I guess we should be using |
You mean |
|
You would also need to remove the |
|
With diff like this: The build passed. |
|
Uploaded PR #4187 |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In the
fstringconstructor from a type that is convertible to string view, thesargument is passed to two different functions, which both internally convert it to astring_view. Thissargument is frequently created by theFMT_STRINGmacro which has a conversion tostring_viewthat evaluates a string literal expression.Each evaluation of a string literal expression can produce a different pointer. Therefore it is not correct to assume that the two conversions of
sto astring_viewproduce relatedstring_views, but the code does assume this. Historically, compilers have let you get away with this in compile-time evaluation, but clang recently gained a more strict implementation of this rule and now rejects.I think the fix is probably to do the same thing in this constructor that is done a few lines below in the next constructor:
template <typename S, FMT_ENABLE_IF(std::is_convertible<const S&, string_view>::value)> FMT_CONSTEVAL FMT_ALWAYS_INLINE fstring(const S& s) : str(s) { if (FMT_USE_CONSTEVAL) + FMT_CONSTEXPR auto sv = string_view(S()); - detail::parse_format_string<char>(s, checker(s, arg_pack())); + detail::parse_format_string<char>(sv, checker(sv, arg_pack())); #ifdef FMT_ENFORCE_COMPILE_STRING static_assert( FMT_USE_CONSTEVAL && sizeof(s) != 0, "FMT_ENFORCE_COMPILE_STRING requires format strings to use FMT_STRING"); #endif } template <typename S, FMT_ENABLE_IF(std::is_base_of<detail::compile_string, S>::value&& std::is_same<typename S::char_type, char>::value)> FMT_ALWAYS_INLINE fstring(const S&) : str(S()) { FMT_CONSTEXPR auto sv = string_view(S()); FMT_CONSTEXPR int ignore = (parse_format_string(sv, checker(sv, arg_pack())), 0); detail::ignore_unused(ignore); }The text was updated successfully, but these errors were encountered: