Internally identify interceptions by content hash#76344
Internally identify interceptions by content hash#76344RikkiGibson merged 2 commits intodotnet:mainfrom
Conversation
| private sealed class InterceptorKeyComparer : IEqualityComparer<(ImmutableArray<byte> ContentHash, int Position)> | ||
| { | ||
| private InterceptorKeyComparer() { } | ||
| public static InterceptorKeyComparer Instance { get; } = new InterceptorKeyComparer(); |
| public int GetHashCode((ImmutableArray<byte> ContentHash, int Position) obj) | ||
| { | ||
| return Hash.Combine( | ||
| BinaryPrimitives.ReadInt32LittleEndian(obj.ContentHash.AsSpan()), |
There was a problem hiding this comment.
nit: FWIW, I was curious about this, so did some exploration. I noticed that we have another IEqualityComparer involving Immutable<byte> (see ByteSequenceBoolTupleComparer). It uses Hash.Combine(ByteSequenceComparer.GetHashCode(.... It's used to cache small method bodies.
In contrast, BinaryPrimitives.ReadInt32LittleEndian truncates most of the content hash. We also use that method in other places of the compiler (see ContentHashComparer)
Just wanted to mention it, but it feels like we're okay with this method for interceptors since the bytes are already a content hash. #Resolved
There was a problem hiding this comment.
I should add a comment that similar implementation is used for InterceptableLocation (the type which holds this data on the "production"/source-generator side)
jcouv
left a comment
There was a problem hiding this comment.
LGTM Thanks (iteration 1) with a nit to consider
Closes #76341
Tests that would be impacted by this change are indicated by ddc9b84 (i.e. the workaround of specifying path in that commit would no longer be necessary.)