Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compilation errors due to make_format_args in gcc 14.1.1 with c++20 #4042

Merged
merged 8 commits into from
Jul 3, 2024
Merged

Fix compilation errors due to make_format_args in gcc 14.1.1 with c++20 #4042

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bd87e0f
use fmt::make_format_args in make_print_args
toge Jul 1, 2024
4a2621d
add test condition
toge Jul 2, 2024
66aaa2e
fix configure error on older gcc
toge Jul 2, 2024
34e5eaa
remove gradle files
toge Jul 2, 2024
0949bbc
revert unnecessary codes
toge Jul 2, 2024
027e6f0
revert forcing cxx_std_20 codes
toge Jul 2, 2024
dc88147
revert deleting likne
toge Jul 2, 2024
71f9d27
improve conditions
toge Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/fmt/printf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,8 @@ using wprintf_args = basic_format_args<wprintf_context>;
/// arguments and can be implicitly converted to `printf_args`.
template <typename Char = char, typename... T>
inline auto make_printf_args(T&... args)
-> decltype(make_format_args<basic_printf_context<Char>>(args...)) {
return make_format_args<basic_printf_context<Char>>(args...);
-> decltype(fmt::make_format_args<basic_printf_context<Char>>(args...)) {
return fmt::make_format_args<basic_printf_context<Char>>(args...);
}

template <typename Char> struct vprintf_args {
Expand Down
Binary file added support/.gradle/8.1.1/checksums/checksums.lock
View file
Open in desktop
Binary file not shown.
Binary file added support/.gradle/8.1.1/checksums/md5-checksums.bin
View file
Open in desktop
Binary file not shown.
Binary file added support/.gradle/8.1.1/checksums/sha1-checksums.bin
View file
Open in desktop
Binary file not shown.
Binary file added support/.gradle/8.1.1/dependencies-accessors/dependencies-accessors.lock
View file
Open in desktop
Binary file not shown.
Empty file added support/.gradle/8.1.1/dependencies-accessors/gc.properties
View file
Open in desktop
Empty file.
Binary file added support/.gradle/8.1.1/fileChanges/last-build.bin
View file
Open in desktop
Binary file not shown.
Binary file added support/.gradle/8.1.1/fileHashes/fileHashes.lock
View file
Open in desktop
Binary file not shown.
Empty file added support/.gradle/8.1.1/gc.properties
View file
Open in desktop
Empty file.
Binary file added support/.gradle/buildOutputCleanup/buildOutputCleanup.lock
View file
Open in desktop
Binary file not shown.
2 changes: 2 additions & 0 deletions support/.gradle/buildOutputCleanup/cache.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#Tue Jul 02 00:25:24 JST 2024
gradle.version=8.1.1
Empty file added support/.gradle/vcs-1/gc.properties
View file
Open in desktop
Empty file.
6 changes: 5 additions & 1 deletion test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ function(add_fmt_test name)
endif ()
add_fmt_executable(${name} ${sources})
target_link_libraries(${name} ${libs})

# use C++20 if C++20 is available for https://github.com/fmtlib/fmt/pull/4042
get_property(cxx_features GLOBAL PROPERTY CMAKE_CXX_KNOWN_FEATURES)
if(cxx_std_20 IN_LIST cxx_features)
target_compile_features(${name} PRIVATE cxx_std_20)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This standard should be controlled externally, not here. Let's revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitaut
Sorry I try to revert both cxx_std and .gradle files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitaut
Now I finished.
Cloud you review it again?

# Define if certain C++ features can be used.
if (FMT_PEDANTIC)
target_compile_options(${name} PRIVATE ${PEDANTIC_COMPILE_FLAGS})
Expand Down
6 changes: 6 additions & 0 deletions test/printf-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
// For the license information refer to format.h.

#include "fmt/printf.h"
// include <format> if possible for https://github.com/fmtlib/fmt/pull/4042
#if defined(__has_include) && __has_include(<format>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

#if FMT_HAS_INCLUDE(<format>) && FMT_CPLUSPLUS > 201703L

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phprus
Thanks a lot!
Fixed.

# include <format>
#endif

#include <cctype>
#include <climits>
Expand Down Expand Up @@ -554,6 +558,8 @@ TEST(printf_test, fixed_large_exponent) {
}

TEST(printf_test, make_printf_args) {
const std::string thing("World");
fmt::printf("PRINTF: Hello, %s!\n", thing);
int n = 42;
EXPECT_EQ("[42] something happened",
fmt::vsprintf(fmt::string_view("[%d] %s happened"),
Expand Down
Loading