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

PlotFileUtil: Add Direct Includes #3446

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Jul 24, 2023

Summary

Adds the used direct includes to AMReX_PlotFileUtil.H.

Additional background

AMReX-Codes/pyamrex#153

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

Addes include correctness to `AMReX_PlotFileUtil.H`.
@WeiqunZhang
Copy link
Member

Correctness here is subjective. I think all those headers have been indirectly included. Are you seeing real issues?

@ax3l
Copy link
Member Author

ax3l commented Jul 27, 2023

Correctness in the sense of include stability:
This will make it long-term stable if transitive headers change.
https://include-what-you-use.org

Changing the label appropriately to cleaning.

@ax3l ax3l added cleaning and removed bug labels Jul 27, 2023
@ax3l ax3l changed the title PlotFileUtil: Add Missing Includes PlotFileUtil: Add Direct Includes Jul 27, 2023
@WeiqunZhang
Copy link
Member

I tried include-what-you-use in the past. It is too much work to make it happy because we have a lot of build options and we need to add a lot of IWYU pragmas. If we were using it, we would have marked AMReX_MultiFab.H as exporting AMReX_Box.H, AMReX_REAL.H etc.

@ax3l
Copy link
Member Author

ax3l commented Aug 1, 2023

I tried include-what-you-use in the past.

Agreed (for the tool). I was more referring to the concept :)

@WeiqunZhang
Copy link
Member

I agree with the concept too. :)

@WeiqunZhang WeiqunZhang merged commit ebe604a into AMReX-Codes:development Aug 1, 2023
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants