search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.91k stars 4.63k forks source link

Do not use ReferenceEquals with impossible types #37691

Open drewnoakes opened 4 years ago

drewnoakes commented 4 years ago

Similar to #33770.

Warn if use of ReferenceEquals would provably always be false based upon the types of the arguments.

Category: Reliability

john-h-k commented 4 years ago

apart from the obvious 

ReferenceEquals(type, typeThatIsntDerivedOrBaseOfOtherType)

would this also apply to value types? Given

ReferenceEquals(someStruct, someStruct)

will always be false currently because they are boxed to different instances. Although, if boxing pooling happened, this could change

stephentoub commented 4 years ago

would this also apply to value types?

That now exists: https://github.com/dotnet/roslyn-analyzers/blob/master/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/DoNotUseReferenceEqualsWithValueTypes.cs

stephentoub commented 4 years ago

cc: @bartonjs

joperezr commented 4 years ago

@stephentoub do we usually keep issues like this open here or do we usually move them to roslyn repo?

bartonjs commented 4 years ago

@joperezr Analyzer proposals for BCL API live in this repository, so we can review them like API proposals.

But this one was already implemented, as #33766.

joperezr commented 4 years ago

I thought the one you pointed at (also @stephentoub pointed at) was only for value types, and this issue was for both that and reference types. Should we reopen to track that part?

bartonjs commented 4 years ago

Oh, right, yeah. That made sense to me at the time, too. Today, however, I got confused.