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.

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

Reintroduce constexpr fmt::formatted_size for C++20 #4103

Merged
merged 2 commits into from
Aug 3, 2024

Conversation

phprus
Copy link
Contributor

@phprus phprus commented Aug 2, 2024

Fix for #4102

FMT_DETAIL_EXPLICIT_INSTANTIATIONS is need to fix visual studio 2019 error:

D:\a\fmt\fmt\include\fmt\base.h(1198,21): error C2672: 'fmt::v11::detail::buffer<T>::append': no matching overloaded function found [D:\a\fmt\build\fmt.vcxproj]
...
D:\a\fmt\fmt\include\fmt\base.h(1198,1): error C2893: Failed to specialize function template 'void fmt::v11::detail::buffer<T>::append(const U *,const U *)' [D:\a\fmt\build\fmt.vcxproj]
          with
          [
              T=char
          ]
D:\a\fmt\fmt\include\fmt\base.h(929): message : see declaration of 'fmt::v11::detail::buffer<T>::append' [D:\a\fmt\build\fmt.vcxproj]
          with
          [
              T=char
          ]
D:\a\fmt\fmt\include\fmt\base.h(1198,1): message : With the following template arguments: [D:\a\fmt\build\fmt.vcxproj]
D:\a\fmt\fmt\include\fmt\base.h(1198,1): message : 'U=Char' [D:\a\fmt\build\fmt.vcxproj]

https://github.com/phprus/fmt/actions/runs/10221349236/job/28283689340

Comment on lines 932 to 934
#ifndef FMT_DETAIL_EXPLICIT_INSTANTIATIONS
FMT_CONSTEXPR20
#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 looks weird and probably a (benign) ODR violation. Maybe we should disable FMT_CONSTEXPR20 completely on this version of MSVC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Visual studio 2019 supports C++20 and I think disabling FMT_CONSTEXPR20 for it is a bad idea.
I suggest disable FMT_CONSTEXPR20 only for fmt::formatted_size (implemented in new commit).

But, I don't see any ODR violation here, since the function body is the same.
If ODR is violated after PR, it must be violated without it due to explicit instantiation

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the new version is cleaner, let's go with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another solution, but it will break library ABI: remove explicit instantiations for buffer<>::append.

Version with disabled FMT_CONSTEXPR20 for fmt::formatted_size on Visual studio 2019 is ready for review.

@phprus phprus marked this pull request as draft August 3, 2024 11:07
include/fmt/base.h Outdated Show resolved Hide resolved
include/fmt/compile.h Outdated Show resolved Hide resolved
phprus added 2 commits August 3, 2024 18:44
Signed-off-by: Vladislav Shchapov <vladislav@shchapov.ru>
Signed-off-by: Vladislav Shchapov <vladislav@shchapov.ru>
@phprus phprus force-pushed the constexpr-fmt_size-2 branch from 67eb7ed to 79abecc Compare August 3, 2024 13:46
@phprus phprus marked this pull request as ready for review August 3, 2024 13:55
@vitaut vitaut merged commit 5ee14d3 into fmtlib:master Aug 3, 2024
43 checks passed
@vitaut
Copy link
Contributor

vitaut commented Aug 3, 2024

Merged, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants