stride
stride copied to clipboard
Address hundreds of compiler warnings
Release Type: GitHub
Version: master
Platform(s): build
Describe the bug When building the main stride solutions, there are hundreds of warnings emitted by the various tools. As of this issue submission, there are ~290 such warnings. This is fairly dangerous as it leads to developers ignoring potential bugs.
As examples of potential issues:
- warning CS8073: The result of the expression is always 'false' since a value of type 'Guid' is never equal to 'null' of type 'Guid?'
- warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
- warning C4244: 'argument': conversion from 'float' to 'const fbxsdk::FbxLongLong', possible loss of data
These may be benign but perhaps not. The safest solution is to examine each and address the warning in an appropriate way.
Additionally, there are reasonably harmless messages but point at clutter that needs to be addressed to streamline the codebase. Things like:
- warning CS0162: Unreachable code detected
- The variable 'e' is declared but never used
- The field 'foo' is never used
- 'some class or field' is obsolete
And the last class is use of deprecated items that the compiler points out. Deprecated classes are usually replaced by something that is superior in some way (for example, security or perf are high value reasons). Granted, these are likely benign at the moment, but could come back to cause issues later.
See also #1381
To Reproduce Steps to reproduce the behavior:
- Compile the stride.sln from the latest master code.
Expected behavior No warnings on build. Bonus would be that warnings are treated as errors.
Screenshots N/A
Log and callstacks See above for examples
Additional context I realize that these might be time consuming to address completely and I am willing to take these on over a series of PRs. From experience, having someone scrutinize these and bring up interesting cases for discussion is well worth the effort.
I see there's even more warnings on the build server. There's SA* warnings - which leads to believe the target that turns off analyzers by default is actually disabled on the build server.
From experience, some of those analyzer warnings are really good, too. Shame they're off locally. I'll have to take a peek at them one day.