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

Fix for FMT_MODULE not compiling on GCC #3605

Merged
merged 4 commits into from
Aug 25, 2023
Merged

Conversation

MathewBensonCode
Copy link
Contributor

@MathewBensonCode MathewBensonCode commented Aug 22, 2023

Removing the friend function declaration has made compilation with FMT_MODULE succeed on GCC on linux. Also Tested with clang on linux.

However I have noted some failed checks with windows.

Perhaps a macro could now fix #3587

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.

Let's make this function not a friend and change the implementation to not access member variables (it should be very easy to do).

- Remove umul128 friend function from uint128_fallback class using
  non-const member access functions instead.
@MathewBensonCode
Copy link
Contributor Author

I hope this is what you had in mind.

Comment on lines 1492 to 1494
auto result = uint128_fallback();
result.lo_ = _umul128(x, y, &result.hi_);
result.low() = _umul128(x, y, &result.high());
return result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change this to something like:

auto hi = uint64_t();
auto lo = _umul128(x, y, &hi);
return {hi, lo};

and remove the new accessors.

- Removal of direct access to members of uint128_fallback and instead
  create the values at the callsite and pass them via the constructor of
  uint128_fallback on the return statement.
@vitaut vitaut merged commit c9efd89 into fmtlib:master Aug 25, 2023
40 checks passed
@vitaut
Copy link
Contributor

vitaut commented Aug 25, 2023

Thank you!

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.

Can't build as C++20 module since 10.1.0
2 participants