-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Closed
dotnet/roslyn-analyzers
#6838Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-System.Runtimecode-analyzerMarks an issue that suggests a Roslyn analyzerMarks an issue that suggests a Roslyn analyzer
Milestone
Description
The following code
var array = new char[0];
var span = array.AsSpan();
var emptyArray = Array.Empty<char>();
var emptySpan = Span<char>.Empty;
if (array == null)
Console.WriteLine("array is null");
else
Console.WriteLine("array is not null");
if (span == null)
Console.WriteLine("span is null");
else
Console.WriteLine("span is not null");
if (emptyArray == null)
Console.WriteLine("emptyArray is null");
else
Console.WriteLine("emptyArray is not null");
if (emptySpan == null)
Console.WriteLine("emptySpan is null");
else
Console.WriteLine("emptySpan is not null");outputs the following:
array is not null
span is not null
emptyArray is not null
emptySpan is null
The only surprising one is Span<char>.Empty == null resulting in true.
Spans can be compared to null because there is an implicit conversion from arrays to spans and the null literal can be converted to an array, so the compiler ends up emitting this:
if (Span<char>.op_Equals(emptySpan, (Span<char>)((char[])null)))
Console.WriteLine("emptySpan is null");
else
Console.WriteLine("emptySpan is not null");The empty span is defined as the null literal converted to a span, so this resulting in true is, albeit surprising, sensible and by design.
This effectively hides the built-in diagnostic CS8073 against comparing value types to null. We should consider adding a diagnostic for comparisons between spans and null.
As pointed out, this should probably also result in a warning:
Span<Cell> line = null;More context on Twitter.
Wraith2, huoyaoyuan, MichaelRumpler, martincostello, Neme12 and 2 more
Metadata
Metadata
Assignees
Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-System.Runtimecode-analyzerMarks an issue that suggests a Roslyn analyzerMarks an issue that suggests a Roslyn analyzer