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 more warnings and add CI for building 3d with configure #1184

Merged
merged 9 commits into from
Jul 24, 2020

Conversation

WeiqunZhang
Copy link
Member

No description provided.

@WeiqunZhang WeiqunZhang requested review from atmyers and ax3l July 24, 2020 02:58
@WeiqunZhang WeiqunZhang force-pushed the fix_warnings branch 5 times, most recently from f32cd2f to fa70006 Compare July 24, 2020 06:12
@@ -179,5 +179,19 @@ jobs:
- name: Build & Install
run: |
./configure --dim 2
make -j2
make -j2 WARN_ALL=TRUE
Copy link
Member

Choose a reason for hiding this comment

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

do you also want to add this to the 1D case above? :)

Copy link
Member

@ax3l ax3l Jul 24, 2020

Choose a reason for hiding this comment

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

Another idea: we can add -Werror in CI through XTRA flags to check PRs do not introduce warnings for selected compilers :)

Copy link
Member Author

Choose a reason for hiding this comment

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

There are too many warnings in 1D we need to fix. :( We do not fully support 1D. So in many places we just have a function with empty body. As for -Werror, we still have some warnings. One we get rid of them, we can add that to a few CI tests.

Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Awesome, thanks a lot!

Src/Base/AMReX.H Outdated
@@ -89,6 +89,10 @@ namespace amrex
void ExecOnFinalize (PTR_TO_VOID_FUNC);
void ExecOnInitialize (PTR_TO_VOID_FUNC);

//! This shuts up the compiler about unused variables
template <class... Ts> AMREX_GPU_HOST_DEVICE
Copy link
Member

@ax3l ax3l Jul 24, 2020

Choose a reason for hiding this comment

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

I usually also put a force inline on that one and make it a constexpr.
Took inspiration from boost:

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good. Will update.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like we have take constexpr, because conexpr void is not supported in C++11.

Copy link
Member

@ax3l ax3l Jul 24, 2020

Choose a reason for hiding this comment

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

Ah right, in C++11 mode I used a const fallback originally: alpaka-group/alpaka#582

@atmyers atmyers merged commit 1fe53cb into AMReX-Codes:development Jul 24, 2020
maximumcats added a commit to maximumcats/amrex that referenced this pull request Jul 26, 2020
WeiqunZhang pushed a commit that referenced this pull request Jul 26, 2020
* Fix CellConservativeProtected bug from #1184

* Fix comment
dwillcox pushed a commit to dwillcox/amrex that referenced this pull request Oct 3, 2020
…des#1184)

* fix more warnings and add CI for building 3d with configure

* make ignore_unused constexpr and force inline

* add AMREX_FALLTHROUGH

* fix more warnings

* enable xsdk in ci and test no_fortran

* remove constexpr from ignore_unused for C++11

* use Clang for 2d ci

* disable mpi for the 2d ci with clang
dwillcox pushed a commit to dwillcox/amrex that referenced this pull request Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants