KEMBAR78
Use native c++ module support from CMake by yujincheng08 · Pull Request #3991 · fmtlib/fmt · GitHub
Skip to content

Conversation

@yujincheng08
Copy link
Contributor

also fix some clang compilation issues when using c++ modules

close #3990

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Overall looks good, just two minor comments inline.

extern template FMT_API auto decimal_point_impl(locale_ref) -> wchar_t;
#endif // FMT_HEADER_ONLY

FMT_END_EXPORT
Copy link
Contributor

Choose a reason for hiding this comment

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

Why exclude native_formatter<T, Char, TYPE>::format from export? If you get an error please post it here for future reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error is:

In file included from fmt/src/fmt.cc:99:
In file included from fmt/include/fmt/args.h:17:
In file included from fmt/include/fmt/args.h:17:
fmt/include/fmt/format.h:4293:64: error: cannot export 'format' as it is not at namespace scope
 4293 | FMT_CONSTEXPR FMT_INLINE auto native_formatter<T, Char, TYPE>::format(
      |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
fmt/include/fmt/format.h:4295:7: error: invalid use of non-static data member 'specs_'
fmt/include/fmt/format.h:4295:7: error: 'specs_' is a private member of 'native_formatter<T, Char, TYPE>'
fmt/include/fmt/base.h:2796:30: note: declared private here
 4295 |   if (specs_.width_ref.kind == arg_id_kind::none &&
 2796 |   dynamic_format_specs<Char> specs_;
      |       ^~~~~~
      |                              ^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe useful: llvm/llvm-project#61890

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So if you wanna export native_formatter<T, Char, TYPE>::format you should export native_formatter<T, Char, TYPE> instead. I tried and it compiled. If you think it okay I can commit it.

@yujincheng08 yujincheng08 requested a review from vitaut June 6, 2024 02:14
@vitaut vitaut merged commit 5c445bc into fmtlib:master Jun 6, 2024
@vitaut
Copy link
Contributor

vitaut commented Jun 6, 2024

Merged, thanks!

@yujincheng08 yujincheng08 deleted the modules branch June 6, 2024 05:29
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.

Use native c++ module support from CMake

2 participants