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

Allow OpenMC to compile against fmt v9 #2136

Merged
merged 1 commit into from
Jul 26, 2022
Merged

Conversation

andrsd
Copy link
Contributor

@andrsd andrsd commented Jul 23, 2022

This fixes the build of OpenMC against fmt v9.x.

@andrsd
Copy link
Contributor Author

andrsd commented Jul 23, 2022

Without this patch, the build fails with:

openmc/src/geometry.cpp:314:12: note: in instantiation of function template specialization 'fmt::format<int &, int &, int &, int &, openmc::Position &>' requested here
      fmt::format("    Crossing lattice {}. Current position ({},{},{}). r={}",

and also on:

openmc/src/output.cpp:196:10: note: in instantiation of function template specialization 'fmt::print<openmc::Position &>' requested here
    fmt::print("    r = {}\n", p.coord(i).r);

on develop branch.

The position structure is printed as (x, y, z). IDK if that's the format you would prefer, but can be easily changed.

I triggered this problem when cmake picked up my fmt library installed via conda on a MacOS.

Let me know if you need any additional information.

Copy link
Contributor

@paulromano paulromano 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 fix @andrsd! We use clang-format for our C++ code (there's a .clang-format file in the root of the repo). To make things easy, I've given a suggested change here with the results of applying clang-format. Once that's updated, I'll trigger CI and assuming all is good we can merge.

include/openmc/position.h Outdated Show resolved Hide resolved
@paulromano paulromano merged commit a4d6de1 into openmc-dev:develop Jul 26, 2022
@andrsd andrsd deleted the fmt-v9 branch July 26, 2022 15:02
@andrsd
Copy link
Contributor Author

andrsd commented Jul 26, 2022

Thanks for merging and your help with getting this into upstream!

@paulromano
Copy link
Contributor

Absolutely, and thank you for helping to improve the code!

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