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

SCons: Fix MSVC bypassing disabled warnings #100185

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Dec 9, 2024

Merging #94321 exposed a bug in our SCons setup on MSVC: some warnings persist even when they should've been disabled via /w. Technically, this is more of an MSVC bug, but this was fixed by implementing these two changes:

  1. Ensuring that the only flags removed in disable_warnings are the flags that change the overall warning scope
  2. When warning=no is passed, ensure that /wd4267 is explicitly appended alongside /w

This includes a few other changes that were integrated while getting to the bottom of this, as they were tangentally related:

  • csg module now correctly rebuilds itself if third-party libraries change
  • Removed redundant MSVC pragma warning suppressions
  • Update GDExtension disable_warnings functions to their main equivalent

@Repiteo Repiteo added this to the 4.4 milestone Dec 9, 2024
@Repiteo Repiteo requested review from a team as code owners December 9, 2024 00:54
@Repiteo Repiteo removed request for a team December 9, 2024 00:54
self["CFLAGS"] = [x for x in self["CFLAGS"] if not (x.startswith("/W") or x.startswith("/w"))]
self["CXXFLAGS"] = [x for x in self["CXXFLAGS"] if not (x.startswith("/W") or x.startswith("/w"))]
WARN_FLAGS = ["/Wall", "/W4", "/W3", "/W2", "/W1", "/W0"]
self["CCFLAGS"] = [x for x in self["CCFLAGS"] if x not in WARN_FLAGS]
Copy link
Member

Choose a reason for hiding this comment

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

Won't this lead to warnings if you have all the disabled_warnings from SCons + /w?
MSVC seems to be really finicky about having any kind of conflicting or double specification of expected behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Surprisingly, no! The conflict warnings only crop up for the main overrides; individual warnings & even warn-as-error can still be passed without issue

@Repiteo Repiteo force-pushed the scons/fix-thirdparty-warnings branch from f96a400 to 070aeb5 Compare December 9, 2024 17:49
@Repiteo Repiteo merged commit f962fdc into godotengine:master Dec 10, 2024
20 checks passed
@Repiteo Repiteo deleted the scons/fix-thirdparty-warnings branch December 10, 2024 20:17
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.

2 participants