-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Suppress IDE0060 breaking local builds #119772
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR suppresses an IDE0060 warning (unused parameter) for the seed
parameter in the HashCodeBuilder
constructor. The author notes this appears to be a bug since the parameter is not used in the implementation.
Key Changes
- Adds pragma directives to suppress IDE0060 warning around the constructor
- No functional code changes
Introduced in #119385, replacing the previous implementation: public HashCodeBuilder(string seed)
{
_hash1 = 0x6DA3B944;
_hash2 = 0;
_numCharactersHashed = 0;
Append(seed);
} |
@jkotas can this get merged? It looks like it's causing a lot of failures in CI. @MichalStrehovsky looks offline. |
I'm open to also adding the missing Append() call since it looks like that's the correct fix. |
It does look like the correct fix. As far as I can tell, it doesn’t currently impact behavior, since the |
Maybe we can add it under a condition checking the seed is not empty? or if |
It makes no difference to behavior: runtime/src/libraries/Common/src/Internal/VersionResilientHashCode.cs Lines 30 to 33 in 2bd4439
|
Let's call Append anyway then. |
Done |
Should I ba-g to unblock people? I verified it builds locally but haven't run tests. |
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
All the call sites are inside an internal class. I think there is fairly obviously no change in behavior, see grep output for src/coreclr/tools/Common/Internal/Metadata/NativeFormat/MetadataTypeHashingAlgorithms.cs
6:using HashCodeBuilder = Internal.VersionResilientHashCode.HashCodeBuilder;
14: private static void AppendNamespaceHashCode(ref HashCodeBuilder builder, NamespaceDefinitionHandle namespaceDefHandle, MetadataReader reader, bool appendDot)
35: private static void AppendNamespaceHashCode(ref HashCodeBuilder builder, NamespaceReferenceHandle namespaceRefHandle, MetadataReader reader, bool appendDot)
60: HashCodeBuilder builder = new HashCodeBuilder(""u8);
82: HashCodeBuilder builder = new HashCodeBuilder(""u8);
src/libraries/Common/src/Internal/VersionResilientHashCode.cs
17: public struct HashCodeBuilder
23: public HashCodeBuilder(ReadOnlySpan<byte> seed) |
/ba-g Unblocking CI, change is behavior-preserving |
Not sure why this unused parameter is here, though. Looks like a bug that it's not used?