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.

Sign up for GitHub

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
Merged

SCons: Fix MSVC bypassing disabled warnings #100185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 26 additions & 27 deletions SConstruct
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,37 +815,36 @@ elif env.msvc:

# Configure compiler warnings
if env.msvc and not methods.using_clang(env): # MSVC
if env["warnings"] == "no":
env.Append(CCFLAGS=["/w"])
else:
if env["warnings"] == "extra":
env.Append(CCFLAGS=["/W4"])
elif env["warnings"] == "all":
# C4458 is like -Wshadow. Part of /W4 but let's apply it for the default /W3 too.
env.Append(CCFLAGS=["/W3", "/w34458"])
elif env["warnings"] == "moderate":
env.Append(CCFLAGS=["/W2"])
# Disable warnings which we don't plan to fix.

env.Append(
CCFLAGS=[
"/wd4100", # C4100 (unreferenced formal parameter): Doesn't play nice with polymorphism.
"/wd4127", # C4127 (conditional expression is constant)
"/wd4201", # C4201 (non-standard nameless struct/union): Only relevant for C89.
"/wd4244", # C4244 C4245 C4267 (narrowing conversions): Unavoidable at this scale.
"/wd4245",
"/wd4267",
"/wd4305", # C4305 (truncation): double to float or real_t, too hard to avoid.
"/wd4324", # C4820 (structure was padded due to alignment specifier)
"/wd4514", # C4514 (unreferenced inline function has been removed)
"/wd4714", # C4714 (function marked as __forceinline not inlined)
"/wd4820", # C4820 (padding added after construct)
]
)
# Disable warnings which we don't plan to fix.
disabled_warnings = [
"/wd4100", # C4100 (unreferenced formal parameter): Doesn't play nice with polymorphism.
"/wd4127", # C4127 (conditional expression is constant)
"/wd4201", # C4201 (non-standard nameless struct/union): Only relevant for C89.
"/wd4244", # C4244 C4245 C4267 (narrowing conversions): Unavoidable at this scale.
"/wd4245",
"/wd4267",
"/wd4305", # C4305 (truncation): double to float or real_t, too hard to avoid.
"/wd4324", # C4820 (structure was padded due to alignment specifier)
"/wd4514", # C4514 (unreferenced inline function has been removed)
"/wd4714", # C4714 (function marked as __forceinline not inlined)
"/wd4820", # C4820 (padding added after construct)
]

if env["warnings"] == "extra":
env.Append(CCFLAGS=["/W4"] + disabled_warnings)
elif env["warnings"] == "all":
# C4458 is like -Wshadow. Part of /W4 but let's apply it for the default /W3 too.
env.Append(CCFLAGS=["/W3", "/w34458"] + disabled_warnings)
elif env["warnings"] == "moderate":
env.Append(CCFLAGS=["/W2"] + disabled_warnings)
else: # 'no'
# C4267 is particularly finicky & needs to be explicitly disabled.
env.Append(CCFLAGS=["/w", "/wd4267"])

if env["werror"]:
env.Append(CCFLAGS=["/WX"])
env.Append(LINKFLAGS=["/WX"])

else: # GCC, Clang
common_warnings = []

Expand Down
2 changes: 1 addition & 1 deletion drivers/d3d12/d3d12ma.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
#endif

#if defined(_MSC_VER)
#pragma warning(disable : 4189 4324 4505)
#pragma warning(disable : 4189 4505)
#endif

#include "thirdparty/d3d12ma/D3D12MemAlloc.cpp"
7 changes: 4 additions & 3 deletions methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,10 @@ def disable_warnings(self):
if self.msvc and not using_clang(self):
# We have to remove existing warning level defines before appending /w,
# otherwise we get: "warning D9025 : overriding '/W3' with '/w'"
self["CCFLAGS"] = [x for x in self["CCFLAGS"] if not (x.startswith("/W") or x.startswith("/w"))]
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

self["CFLAGS"] = [x for x in self["CFLAGS"] if x not in WARN_FLAGS]
self["CXXFLAGS"] = [x for x in self["CXXFLAGS"] if x not in WARN_FLAGS]
self.AppendUnique(CCFLAGS=["/w"])
else:
self.AppendUnique(CCFLAGS=["-w"])
Expand Down
21 changes: 13 additions & 8 deletions modules/csg/SCsub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,22 @@ thirdparty_sources = [
]

thirdparty_sources = [thirdparty_dir + file for file in thirdparty_sources]
env_csg.Prepend(
CPPPATH=[
thirdparty_dir + "include",
]
)
env_csg.Prepend(CPPPATH=[thirdparty_dir + "include"])
env_thirdparty = env_csg.Clone()
env_thirdparty.disable_warnings()
env_thirdparty.add_source_files(thirdparty_obj, thirdparty_sources)
env.modules_sources += thirdparty_obj

# Godot's own source files
env_csg.add_source_files(env.modules_sources, "*.cpp")
# Godot source files

module_obj = []

env_csg.add_source_files(module_obj, "*.cpp")

if env.editor_build:
env_csg.add_source_files(env.modules_sources, "editor/*.cpp")
env_csg.add_source_files(module_obj, "editor/*.cpp")

env.modules_sources += module_obj

# Needed to force rebuilding the module files when the thirdparty library is updated.
env.Depends(module_obj, thirdparty_obj)
16 changes: 6 additions & 10 deletions modules/text_server_adv/gdextension_build/methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,13 @@ def disable_warnings(self):
if self["platform"] == "windows" and not self["use_mingw"]:
# We have to remove existing warning level defines before appending /w,
# otherwise we get: "warning D9025 : overriding '/W3' with '/w'"
warn_flags = ["/Wall", "/W4", "/W3", "/W2", "/W1", "/WX"]
self.Append(CCFLAGS=["/w"])
self.Append(CFLAGS=["/w"])
self.Append(CXXFLAGS=["/w"])
self["CCFLAGS"] = [x for x in self["CCFLAGS"] if x not in warn_flags]
self["CFLAGS"] = [x for x in self["CFLAGS"] if x not in warn_flags]
self["CXXFLAGS"] = [x for x in self["CXXFLAGS"] if x not in warn_flags]
WARN_FLAGS = ["/Wall", "/W4", "/W3", "/W2", "/W1", "/W0"]
self["CCFLAGS"] = [x for x in self["CCFLAGS"] if x not in WARN_FLAGS]
self["CFLAGS"] = [x for x in self["CFLAGS"] if x not in WARN_FLAGS]
self["CXXFLAGS"] = [x for x in self["CXXFLAGS"] if x not in WARN_FLAGS]
self.AppendUnique(CCFLAGS=["/w"])
else:
self.Append(CCFLAGS=["-w"])
self.Append(CFLAGS=["-w"])
self.Append(CXXFLAGS=["-w"])
self.AppendUnique(CCFLAGS=["-w"])


def make_icu_data(target, source, env):
Expand Down
16 changes: 6 additions & 10 deletions modules/text_server_fb/gdextension_build/methods.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,13 @@ def disable_warnings(self):
if self["platform"] == "windows" and not self["use_mingw"]:
# We have to remove existing warning level defines before appending /w,
# otherwise we get: "warning D9025 : overriding '/W3' with '/w'"
warn_flags = ["/Wall", "/W4", "/W3", "/W2", "/W1", "/WX"]
self.Append(CCFLAGS=["/w"])
self.Append(CFLAGS=["/w"])
self.Append(CXXFLAGS=["/w"])
self["CCFLAGS"] = [x for x in self["CCFLAGS"] if x not in warn_flags]
self["CFLAGS"] = [x for x in self["CFLAGS"] if x not in warn_flags]
self["CXXFLAGS"] = [x for x in self["CXXFLAGS"] if x not in warn_flags]
WARN_FLAGS = ["/Wall", "/W4", "/W3", "/W2", "/W1", "/W0"]
self["CCFLAGS"] = [x for x in self["CCFLAGS"] if x not in WARN_FLAGS]
self["CFLAGS"] = [x for x in self["CFLAGS"] if x not in WARN_FLAGS]
self["CXXFLAGS"] = [x for x in self["CXXFLAGS"] if x not in WARN_FLAGS]
self.AppendUnique(CCFLAGS=["/w"])
else:
self.Append(CCFLAGS=["-w"])
self.Append(CFLAGS=["-w"])
self.Append(CXXFLAGS=["-w"])
self.AppendUnique(CCFLAGS=["-w"])


def make_icu_data(target, source, env):
Expand Down
9 changes: 0 additions & 9 deletions modules/theora/video_stream_theora.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,8 @@
#include "core/os/os.h"
#include "scene/resources/image_texture.h"

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4127)
#endif

#include "thirdparty/misc/yuv2rgb.h"
Repiteo marked this conversation as resolved.
Show resolved Hide resolved

#ifdef _MSC_VER
#pragma warning(pop)
#endif

int VideoStreamPlaybackTheora::buffer_data() {
char *buffer = ogg_sync_buffer(&oy, 4096);

Expand Down