search

libMesh / TIMPI

Templated Interface to MPI
GNU Lesser General Public License v2.1
12 stars 11 forks source link

prevent gcc spurious warnings #88

Closed BalticPinguin closed 2 years ago

BalticPinguin commented 2 years ago

After updating my compiler and libMesh-library, I got fetched by gcc's spurious warning which was resolved in the analogous libmesh-test.

For some reason, you seemingly never ran into this? (my gcc-version is 11.2.0).

Best regards

moosebuild commented 2 years ago

Job Coverage on 7da73fd wanted to post the following:

Coverage

Coverage did not change

Full coverage report

This comment will be updated on new commits.

jwpeterson commented 2 years ago

I agree it looks like a similar issue, but what I don't understand here is that both the src and dest declarations are identical, so why would we not need to fix both of them to work around the spurious warning?

BalticPinguin commented 2 years ago

I fully agree that it does'nt make much sense. If you want, I can fix both of them to please the readers eye, but for gcc it seems to be enough this way.

Unfortunately, I don't understand that warning in general; but from what it sais I think that the critical part is the dest = src where gcc thinks that dest hasn't enough memory allocated. So it is enough to fix dest to destroy the pattern that creates the false positive.

jwpeterson commented 2 years ago

but from what it sais I think that the critical part is the dest = src

Ah, yeah that sounds familiar, and would be consistent with only needing to update the dest declaration. My only other request would be that we put a comment about "working around a compiler bug" on this line of code, but that can be added just before merge.

roystgnr commented 2 years ago

How did I never run into this? I added a workaround for the exact same warning in tests/parallel/parallel_test.C in libMesh, but didn't hit the issue here at the same time?

I'm happy with the fix here but I'll add a comment before merging; if we can't write standard code because a compiler won't let us then we definitely need to document that lest someone decide to get clever and "fix" it later.

roystgnr commented 2 years ago

Gah, I take back "add a comment before merging". I didn't notice that the PR here is from your master branch, and even if you've enabled maintainer updates to that, it just feels wrong to push them. Would you just add something along the lines of what's in the libMesh workaround?

   // Workaround for spurious warning from operator=
   // https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100366
jwpeterson commented 2 years ago

I'll take care of it. Just need to pull the branch, add the comment and merge by hand. No need to push back to @BalticPinguin's master.