runtime: String == ReadOnlySpan is FALSE; Really Bad Design and Needs to be FIX

I ran into this problem which has cost my company a day’s worth of work.

string b = "bob";
string b2 = "boby";

var s = b2[..^1];
var t = b == s;     // true

var s2 = b2.AsSpan()[..^1];
var t2 = b == s2; // false. 

// How is a developer to know that s2 is of type Span and b is not without tracking down their type declarations? 
// The point is, they shouldn't have too.

Digging deeper, I reviewed the implementation to == and was very shocked and horrified when I read this

        /// <summary>
        /// Returns true if left and right point at the same memory and have the same length.  Note that
        /// this does *not* check to see if the *contents* are equal.
        /// </summary>
        public static bool operator ==(ReadOnlySpan<T> left, ReadOnlySpan<T> right) =>
            left._length == right._length &&
            Unsafe.AreSame<T>(ref left._pointer.Value, ref right._pointer.Value);

What good is a span if its equality operator isn’t transparent? Especially with strings, which I suspect will be the number 1 use-case for spans. In the case of strings, the current implementation does not pass the Duck Test but should.

Until this is fixed, we (all .NET loving developers around the world) will have to know explicitly if we are dealing with a span or not. As in, “To span or not to span, that is the question”. Frankly, the answer should be, who cares? The two equalities are syntactically the same, and thus they should be semantically the same as well.

Whereas, comparing two arrays of ints, for example, doesn’t currently have the same syntactical or semantic meaning. Thus as developers, we wouldn’t expect element-wise but pointer comparison in this case.

This issue #10151, first addressed this problem back in 2018, which should have been fixed way back then.

@Microsoft: How many developers, and their hidden bugs, need to fall into this trap before this grievous issue is fixed? Or will this design decision go down in history as the next Billion Dollar Software Language Design Mistake?

Please fix this issue. Add a new project switch or some other backward-compatible flag, we don’t care; we just want it fixed. PLEASE!

Call to all developers, if you agree or disagree, either way please vote on this issue.

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 13
Comments: 15 (9 by maintainers)

Most upvoted comments

I may be wrong, but I have a hard time believing that anders Hejlsbery, the senior language architect of C# and of TypeScript, would or has agreed with this current design choice.

While Anders certainly helped start C#, it is not purely his language and he hasn’t been the language architect for C# for many years now. Objectively speaking, the language has had much more overall evolution under the leadership of Mads Torgersen (lead designer for C#) and Jared Parsons (dev lead for C#) than anyone else.

Likewise, .NET extends far beyond the scope of just C# and the CLR does not standard for “C# Language Runtime”. .NET exists as a “Common Language Runtime” for many languages including C#, VB, F#, C++/CLI, IronPython, COBOL.NET, etc…

These decisions are carefully considered within the scope of the entire .NET ecosystem. What works well, what doesn’t work, patterns that developers should or shouldn’t follow, etc. In the case of string, even using == is something you normally don’t want. You should instead explicitly be using Equals(string left, string right, StringComparison comparisonType) so you can correctly account for Ordinal vs CurrentCulture vs InvariantCulture and CaseSensitive vs IgnoreCase. The same applies to Compare, CompareTo, and most other string APIs.

Therefore, can we either:

Neither 1 nor 2 are viable options. Span has shipped the way it has for several releases now and these would be a massively breaking change. An option that you didn’t list is to update your code to call Equals and to correctly pass in StringComparison. We are looking at providing an analyzer (https://github.com/dotnet/runtime/issues/54794) to help flag potential misuse of an API where users potentially desired a string comparison instead.

The decision for == to behave the way it does was with taking into account that:

Not every ROSpan<char> is a string. It could also be a buffer over a char[]
ROSpan<T> encompasses far more than just strings and character buffers. Having one behavior for char and another for say byte or float would be massively confusing.
The typical and recommended use-case for string is to explicitly pass in StringComparison to ensure that the behavior you want is exactly what you get. We already have an unfortunate issue where not all APIs on string behave the same and where sometimes you get ordinal by default and other times you get culture-aware, etc
There were also many more considerations taken into account and it would take far too long to list them all.

tannergooding on Oct 7, 2022

Yes, strings (a variable-length array of chars) are complicated, and yet .NET still deals with them ‘correctly.’ We are not looking for a “general case” here, we are explicitly talking about strings, nothing more.

To be more explicit about @tannergooding 's comment - in the general case for strings, == will produce unwanted or unexpected results. It only produces "correct"results for certain simple cases (mostly just English). If you’re in a server context you should normally avoid it, in preference for additionally being explicit about the culture being used, in addition to the comparison needed.

Clockwork-Muse on Oct 7, 2022

#54794

hrrrrustic on Oct 7, 2022

Closing in favor of https://github.com/dotnet/runtime/issues/54794.

Jozkee on Oct 12, 2022

Yes, strings (a variable-length array of chars) are complicated, and yet .NET still deals with them ‘correctly.’ We are not looking for a “general case” here, we are explicitly talking about strings, nothing more.

To be more explicit about @tannergooding 's comment - in the general case for strings, == will produce unwanted or unexpected results. It only produces "correct"results for certain simple cases (mostly just English). If you’re in a server context you should normally avoid it, in preference for additionally being explicit about the culture being used, in addition to the comparison needed.

Mr0N on Oct 8, 2022