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 maybe-unused warning in grid_tools_dofhandler #16871

Merged
merged 1 commit into from Apr 8, 2024

Conversation

tjhei
Copy link
Member

@tjhei tjhei commented Apr 7, 2024

./include/deal.II/grid/reference_cell.h:2799:22: warning: ‘face1_vertices.std::array<unsigned[763/12022] M_elems[0]’ may be used uninitialized in this function [-Wmaybe-uninitialized]
./source/grid/grid_tools_dof_handlers.cc:2438:7: note: ‘face1_vertices.std::array<unsigned int, 1>::_M_elems[0]’ was declared here

Comment on lines 2437 to 2438
std::array<unsigned int, GeometryInfo<dim>::vertices_per_face>
face1_vertices, face2_vertices;
face1_vertices{}, face2_vertices;
Copy link
Member

Choose a reason for hiding this comment

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

I understand why this patch fixes the warning, but I don't understand why this is the right thing to do. In the code below, down to line 2475, face1_vertices and face2_vertices seem to be written into in always the same places. So it's not clear to me why initializing only one of these but not the other is right.

But really what's unclear to me is why initialization with zero is right. Do we expect every element of the array to be written into? If so, perhaps initialization with an invalid value would be the right choice?

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume the diagnostics just pick up the loop iterating over face1_vertices in the other function, but I don't know for sure.
I don't understand the code fully, but I now fill both with invalid data and check the content at the end. We'll see what happens.

```
./include/deal.II/grid/reference_cell.h:2799:22: warning:
‘face1_vertices.std::array<unsigned[763/12022] M_elems[0]’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
./source/grid/grid_tools_dof_handlers.cc:2438:7: note:
‘face1_vertices.std::array<unsigned int, 1>::_M_elems[0]’ was declared
here
```
Copy link
Member

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

Yes, this is better. I'm confounded by you choosing obj.end() == std::find(...) over std::find(...) == std::end() :-)

@bangerth bangerth merged commit d610841 into dealii:master Apr 8, 2024
16 checks passed
@bangerth bangerth mentioned this pull request Apr 8, 2024
@tjhei tjhei deleted the maybe-uninit-orthogonal branch April 8, 2024 18:05
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.

None yet

4 participants