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

debug consistency: fix unused-but-set-variable warning #360

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

hannesbrandt
Copy link
Contributor

@donnaaboise reported the following unused-but-set-variable warning.

/Users/calhoun/projects/ForestClaw/code/forestclaw/src/fclaw2d_convenience.c:1813:9: warning: variable 'prev_rank' set but not used [-Wunused-but-set-variable]
 1813 |     int prev_rank;
      |         ^~~~~~~~~
[183/1751] Building C object src/CMakeFiles/forestclaw_c.dir/fclaw2d_file.c.o

This PR replaces the call SC_ASSERT (prev_rank < sb->rank); by FCLAW_ASSERT (prev_rank < sb->rank); so that prev_rank is set and used consistently in an FCLAW_ENABLE_DEBUG environment.
I was not able to reproduce the compiler warning. Does this solve the warning, @donnaaboise?

There are a few more occurrences of P4EST_ASSERT and P4EST_ENABLE_DEBUG in fclaw2d_convenience.c, forestclaw2d.c,
fclaw2d_p4est.c and patches/clawpatch/fclaw_clawpatch_output_vtk.c. I could go ahead and replace them with their FCLAW counterpart in another commit, if this is okay with you @donnaaboise @cburstedde. Such a change would prevent issues caused by sc/p4est and forestclaw having different debug configurations.

@scottaiton scottaiton changed the base branch from develop-3d to develop September 20, 2024 15:07
@donnaaboise
Copy link
Member

Yes - please go ahead and make the changes FCLAW_XXX asserts. I assume @cburstedde is okay with this?

@cburstedde
Copy link
Member

Yes - please go ahead and make the changes FCLAW_XXX asserts. I assume @cburstedde is okay with this?

Sounds good, please @hannesbrandt go ahead with the remaining changes.

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.

3 participants