search

DrTimothyAldenDavis / SuiteSparse

The official SuiteSparse library: a suite of sparse matrix algorithms authored or co-authored by Tim Davis, Texas A&M University.
https://people.engr.tamu.edu/davis/suitesparse.html
Other
1.15k stars 259 forks source link

SPEX: Remove pragma that silenced a GCC warning #773

Closed mmuetzel closed 7 months ago

mmuetzel commented 7 months ago

Remove pragma that suppresses a warning that is no longer needed. The Ubuntu runners have GCC 11.4, and there is no warning without this part for them.

ISTR that we refactored the code surrounding mpq. Maybe, that fixed the issue that the warning was pointing to...

DrTimothyAldenDavis commented 7 months ago

I need to keep that pragma. It avoids a bug in gcc 11, which reports a spurious warning. The warning was seen by the reviewers of the SPEX paper and the journal asked us to fix it.

mmuetzel commented 7 months ago

Was that before or after d9c38c46318b232c6080c560ddd53ad6a1fa77ae?

There is no warning with GCC 11 in the CI.

DrTimothyAldenDavis commented 7 months ago

The warning never appeared in the CI here on the SuiteSparse github. It appears in the SPEX repo https://github.com/clouren/SPEX . It might not affect all gcc 11.x versions.

gcc 11.x for some range of x does indeed have a bug that causes this spurious warning so I really do need to keep this pragma.

DrTimothyAldenDavis commented 7 months ago

I need to keep the pragma even if gcc 11.4 doesn't report the spurious error; we were instructed to fix the warning and I don't know what gcc 11.x the reviewers used. I can't risk having the warning reappear for some untested gcc 11.x.y.

mmuetzel commented 7 months ago

That repository doesn't seem to have an equivalent to https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/d9c38c46318b232c6080c560ddd53ad6a1fa77ae.

Are you sure this isn't fixed by that change?

(Imho, suppressing a compiler warning with a pragma is hardly a "fix" honestly.)

Worst thing that could happen is probably that the warning isn't shown for an actual issue. But if you absolutely prefer to keep that pragma, that's your call.

DrTimothyAldenDavis commented 7 months ago

An actual issue would be found by any other version of gcc, just not gcc 11. I agree it's not a satisfying fix, just a workaround for a bug in gcc I can't control.