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

[3.2] Fix const warnings in FBX (build failure on macOS) #45351

Merged
merged 1 commit into from
Jan 21, 2021

Conversation

asmaloney
Copy link
Contributor

@asmaloney asmaloney commented Jan 21, 2021

Fixes several instances of error: 'const' type qualifier on return type has no effect [-Werror,-Wignored-qualifiers]

This looks like it was fixed in 6607fc7 when FBX was merged into the 4.0 master branch:

"- fixed const correctness with C++/C version change"

Fixes several instances of error: 'const' type qualifier on return type has no effect [-Werror,-Wignored-qualifiers]

This looks like it was fixed in 6607fc7
when FBX was merged into the 4.0 master branch:

"- fixed const correctness with C++/C version change"
@asmaloney
Copy link
Contributor Author

I'm not sure why the CI isn't producing these warnings. This is the scons command I'm using, so everything is pretty much the default:

scons target=debug dev=yes tools=yes --jobs=$(sysctl -n hw.logicalcpu)

I'm also unsure why some of the compile options are different between my build and the CI. The compiler versions are the same: Clang 11.0.0 (clang-1100.0.33.17). I am running macOS 10.14.6, not 10.15.

Command line details comparison for compilation of FBXParser.cpp on my computer vs. the CI

Mine

clang++ -o modules/fbx/fbx_parser/FBXParser.osx.tools.64.o -c -std=gnu++14 -Wctor-dtor-privacy -Wnon-virtual-dtor -g3 -arch x86_64 -mmacosx-version-min=10.9 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -Wall -Wextra -Wwrite-strings -Wno-unused-parameter -Werror -DDEBUG_ENABLED -DDEBUG_MEMORY_ALLOC -DDISABLE_FORCED_INLINE -DOSX_ENABLED -DUNIX_ENABLED -DGLES_ENABLED -DAPPLE_STYLE_KEYS -DCOREAUDIO_ENABLED -DCOREMIDI_ENABLED -DGL_SILENCE_DEPRECATION -DPTRCALL_ENABLED -DTOOLS_ENABLED -DGDSCRIPT_ENABLED -DMINIZIP_ENABLED -DZSTD_STATIC_LINKING_ONLY -DGLAD_ENABLED -DGLES_OVER_GL -DMODULE_BMP_ENABLED -DMODULE_BULLET_ENABLED -DMODULE_CAMERA_ENABLED -DMODULE_CSG_ENABLED -DMODULE_CVTT_ENABLED -DMODULE_DDS_ENABLED -DMODULE_DENOISE_ENABLED -DMODULE_ENET_ENABLED -DMODULE_ETC_ENABLED -DMODULE_FBX_ENABLED -Imodules/fbx -Ithirdparty/libpng -Ithirdparty/glad -Ithirdparty/zstd -Ithirdparty/zlib -Iplatform/osx -I. modules/fbx/fbx_parser/FBXParser.cpp

CI

clang++ -o modules/fbx/fbx_parser/FBXParser.osx.opt.tools.64.o -c -std=gnu++14 -g2 -O2 -arch x86_64 -mmacosx-version-min=10.9 -isysroot /Applications/Xcode_12.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.0.sdk -Wall -Werror -DDEBUG_ENABLED -DOSX_ENABLED -DUNIX_ENABLED -DGLES_ENABLED -DAPPLE_STYLE_KEYS -DCOREAUDIO_ENABLED -DCOREMIDI_ENABLED -DGL_SILENCE_DEPRECATION -DPTRCALL_ENABLED -DTOOLS_ENABLED -DGDSCRIPT_ENABLED -DMINIZIP_ENABLED -DZSTD_STATIC_LINKING_ONLY -DGLAD_ENABLED -DGLES_OVER_GL -DMODULE_BMP_ENABLED -DMODULE_BULLET_ENABLED -DMODULE_CAMERA_ENABLED -DMODULE_CSG_ENABLED -DMODULE_CVTT_ENABLED -DMODULE_DDS_ENABLED -DMODULE_DENOISE_ENABLED -DMODULE_ENET_ENABLED -DMODULE_ETC_ENABLED -DMODULE_FBX_ENABLED -Imodules/fbx -Ithirdparty/libpng -Ithirdparty/glad -Ithirdparty/zstd -Ithirdparty/zlib -Iplatform/osx -I. modules/fbx/fbx_parser/FBXParser.cpp

@asmaloney
Copy link
Contributor Author

asmaloney commented Jan 21, 2021

Ah found out why the options are different. dev turns on extra warnings.

    if env["dev"]:
        env["verbose"] = True
        env["warnings"] = "extra"
        env["werror"] = True

These implies that at least one CI should build with dev=yes... or at least warnings=extra.

@akien-mga
Copy link
Member

akien-mga commented Jan 21, 2021

The 3.2 branch is not warning free when using extra, which is why it's not enabled by default. It is however enabled in the master branch which is warning free on all current CI builds.

@akien-mga akien-mga merged commit 2d90412 into godotengine:3.2 Jan 21, 2021
@akien-mga
Copy link
Member

Thanks!

@asmaloney asmaloney deleted the fix-FBX-warnings branch January 21, 2021 22:48
@asmaloney
Copy link
Contributor Author

Oh, OK. When I was building it before (before == several months ago) with dev=yes it was compiling fine.

Anyway, after the three fixes I PR-ed, it compiles with it on for me again.

@akien-mga akien-mga modified the milestones: 3.2, 3.3 Apr 20, 2021
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.

3 participants