GeezerVison

@ericsnowcurrently Please have a look.

I think perhaps we should only add the flag when we detect that the compiler supports it? What do you think?

I think perhaps we should only add the flag when we detect that the compiler supports it? What do you think?

Yes, please add a compiler flag check. FWIW, the autoconf changes themselves look good.

How can we verify (now or in the future) that these modules can't use strict overflow or that everything else can?

Let's discuss this over on gh-96821.

@AA-Turner Thanks for the suggestions! Much appreciated.

Thanks for shepherding this through, @erlend-aasland!

...and @matthiasgoergens for doing the work, of course. 😄

(I'm working my way through some PRs which have been approved and are labeled "awaiting merge", hence my seemingly bolt from the blue comment. Why? Read here.)

This work looks to be completed. Can someone merge?

(I'm working my way through some PRs which have been approved and are labeled "awaiting merge", hence my seemingly bolt from the blue comment. Why? Read here.)

This work looks to be completed. Can someone merge?

I'm awaiting final changes. See #96823 (comment)

@erlend-aasland I was interested in picking this up, so I added the help message change you suggested here #96823 (comment) to the PR

I was looking into adding the warn as well, but I'm not familiar with autoconf. The existing PR looks like it's missing something to me... should we be setting ac_cv_cc_supports_fstrict_overflow in the branches of the AC_COMPILE_IFELSE?

@erlend-aasland I was interested in picking this up, so I added the help message change you suggested here #96823 (comment) to the PR

Great, thanks :)

I was looking into adding the warn as well, but I'm not familiar with autoconf. The existing PR looks like it's missing something to me... should we be setting ac_cv_cc_supports_fstrict_overflow in the branches of the AC_COMPILE_IFELSE?

Yes; if we don't, configure will execute this check for every run, even if we're using the -C flag. Also, we must not set STRICT_OVERFLOW_CFLAGS and NO_STRICT_OVERFLOW_CFLAGS inside the AC_CACHE_CHECK; instead, add a AS_VAR_IF after the check, and set those two variables if ac_cv_cc_supports_fstrict_overflow is yes.

Please pick this up, if you can. Thanks!

I don't have as much time as I would like these days.

Okay, I think this is ready. I'm not familiar with autoconf, so apologies for any issues!

Thanks, Shantanu! It looks good, AFAICS. As an improvement, we could rewrite the logic to only check for the compiler switch if --with-strict-overflow was given.

Gentle ping @hauntsaninja @erlend-aasland This came up in discussion and it looks like we're really close to being -fstrict-overflow clean here. Anything else this needs to be merged for 3.12, besides reviewing and merging #96925 also? From a (minimal) docs review perspective, this looks good. Thanks!

I think this is ready to merge if we need it. Erlend's last comment is a nice to have and I can address it in a follow up PR.

Nice to see this progressing!

Thanks Shantanu and Matthias!