fixed order in SConstruct

[Cyberrebell]

In #63288 the LTO handling was improved but the default value for LTO wouldn't effect the build.

Without my changes if you add production=yes but no lto argument the platform specific configure() function is called with lto=none and will add no lto compiler flags. After configure() lto will be defaulted to full and print "Using LTO: full" but in fact the compiler will run without LTO as the compiler flags were set before.

[akien-mga]

The behavior you describe is intentional, at least on some platforms like Windows MSVC and macOS/iOS/Android where we don't want to enable LTO by default because it's either worse than no LTO, or not tested well enough to prove its benefit.

If you do want to use LTO anyway, you can specify it with the lto option, which should take precedence. The production option is intended to use the sane and tested configuration that we apply to production builds, which do not all use LTO.

[Cyberrebell]

ah I see. I could call detect.get_flags() on the platform look into platform_flags and check if there is an default value ("lto", "none") and additional ignore msvc.
Maybe even add a hint if LTO was enabled manually on one of those platforms.

[akien-mga]

Well the fact that configure() runs first is already used to define what's the intended LTO value to use by default for production=yes on each platform (via the detect.py get_flags()).

So I don't think anything needs to be changed, or I don't understand what the actual problem is that you're trying to solve.

[akien-mga]

Oh I understand the problem now, the LTO config isn't respected. That's a bug indeed.

Probably the dev and production logic needs to go between reading the platform flags (to get platform specific default overrides) and configure, so just before configure.

[Cyberrebell]

ok. I'll try to add a smart platform dependent check before applying the default value for lto

[Cyberrebell]

now this should be better

[akien-mga]

This looks mostly good, but I'm not fond of adding a dedicated method for this which duplicates what is already done in get_flags() for each platform (and for MSVC, we actually rely on the current behavior to warn users about using MSVC for production builds:

        if env.msvc:
            print(
                "WARNING: For `production` Windows builds, you should use MinGW with GCC "
                "or Clang instead of Visual Studio, as they can better optimize the "
                "GDScript VM in a very significant way. MSVC LTO also doesn't work "
                "reliably for our use case."
                "If you want to use MSVC nevertheless for production builds, set "
                "`debug_symbols=no lto=none` instead of the `production=yes` option."
            )
            Exit(255)

So I made #65745 to fix this issue, I think moving the production logic between checking get_flags() and configure() is sufficient to fix it.

[akien-mga]

Reopening for now as I'm not 100% my solution is better, need to dig deeper.

[akien-mga]

I think #65745 should be good, in the end I had to implement something not unlike what you originally attempted. Sorry for coming full circle :D
I think it's clearer now with the auto value and doing the handling in each platform. Would be great if you can review/test it :)

[akien-mga]

Superseded by #65745.