Fix Clang warnings #8634
Fix Clang warnings #8634
Conversation
Fixes 190 warnings of the following kinds, emitted when compiling with Clang 20: - warning: implicit conversion from 'int' to 'float' changes value from 2147483647 to 2147483648 [-Wimplicit-const-int-float-conversion] - warning: variable '...' set but not used [-Wunused-but-set-variable] - warning: first argument in call to 'memset' is a pointer to non-trivially copyable type '...' [-Wnontrivial-memcall]
| auto& sb = m_builder; | ||
|
|
||
| if (const auto incompleteExpr = as<IncompleteExpr>(expr)) | ||
| if (as<IncompleteExpr>(expr)) |
There was a problem hiding this comment.
It may need more discussion about this.
But I treat this as a Slang coding style.
At some point, I think I made some changes to CMake to ignore this warning for Clang.
I am not sure why it didn't work for you.
If we go with fixing the source code rather than snoozing the warning, we will also need to revert my change in CMake.
I personally think that this is a kind of function parameter unused.
void myFunc(int probablyUnused)
{
... `probablyUnused` is not used ...
}
It sometimes helps us to find an unexpected mistake/bug.
And I see a value of having the warning enabled.
But the annoying part is that, when you are making code changes during the development, you have to keep removing them all. Or if you changed the code to use the variable, you have to bring them all again and you have to spend time coming up with a good variable name again.
Alternatively, we may snooze the warning with
(void)incompleteExpr; // We have SLANG_UNUSED macro too
It will be worth bringing up a discussion with more Slang dev members before committing this change.
There was a problem hiding this comment.
At some point, I think I made some changes to CMake to ignore this warning for Clang.
I am not sure why it didn't work for you.
You excluded it from -Werror (which I guess you enabled manually in your local builds) in #7561.
USE_FEWER_WARNINGS adds -Wno-unused-but-set-variable: https://github.com/expipiplus1/slang/blob/27c171ffb1f965df575d9b7c1d7c555263a68fa2/cmake/CompilerFlags.cmake#L135
but not all Slang targets use that.
| for (int i = 0; i < inputCount; i++) | ||
| { | ||
| inputBufferData.push_back((float)rand() / RAND_MAX); | ||
| inputBufferData.push_back(rand() / static_cast<float>(RAND_MAX)); |
There was a problem hiding this comment.
Can we improve this better with a more modern way like std::mt19937?
There was a problem hiding this comment.
Would that be accepted with the "be skeptical of 'modern C++' ideas unless they are clearly better than simpler alternatives" rule?
The lines changed in slang-ir.cpp are. I replaced calls to |
|
I forgot to mention, I also got 6 |
2 participants
Fixes 190 warnings of the following kinds, emitted when compiling with Clang 20: