Add SpecialType entry for ReadOnlySpan

[DoctorKrolic]

Background and Motivation

In order to implement this codegen optimization compiler needs to have ReadOnlySpan as a special type in order to be able to recognize other members of special types, in this case, span-based Concat members of string type. For more info see last paragraph of #71793 (comment).
The new special type can then be used to recognize other special members for compiler optimizations.

Proposed API

namespace Microsoft.CodeAnalysis
{
     public enum SpecialType
     {
+        System_ReadOnlySpan_T = 47,

-        Count = System_Runtime_CompilerServices_InlineArrayAttribute
+        Count = System_ReadOnlySpan_T
     }

Usage Examples

See #71793

Alternative Designs

None

Risks

1. New members to maintain
2. Ease of misuse on consumer's side. SpecialType is only gonna be set for types from the corlib, meaning, that in this particular case checking for SpecialType.System_ReadOnlySpan_T is not an easy check for a canonical definition of ReadOnlySpan, since the type can come from other places, e.g. System.Memory NuGet package. We will document it as such in code, but this doesn't mean that there won't be any misuses of this new API

[CyrusNajmabadi]

I'm happy to bring this to next API review. But i'm curious prior to that if there's a core design reason at the compiler level to not do this. I'm not an expert here, but i thought there were some aspects/rules around when SpecialType was appropriate or not, and i'm curious if that was already considered here.

@AlekseyTs @333fred for thoughts. Or, we can just discuss at the API meeting.

Note: if this was offered, IDE would use it immediately :)

[AlekseyTs]

With the addition of corresponding SpecialType entries this can be simplified down to a single special type check without the need to request additional type from the compilation.

This is incorrect and this is not a motivation for the compiler to add the API for ReadOnlySpan. The entry is not added to provide a way for locating the "official" definition of ReadOnlySpan, it is added specifically to provide a way to locate a definition of the type with the given name in core library. In some scenarios that type is not the type that compiler is going to treat as the "official" span type. And we are not intending to change the mechanism of locating the "official" span type at the moment. Compiler will not be using the new SpecialType entry for this purpose.

At the moment compiler doesn't need to have the same support for Span, and I do not think we should add one. Actually to reduce the confusion and user mistakes.

Note: if this was offered, IDE would use it immediately :)

This is likely undesirable. Compiler supports special handling of span types that are not declared in the core library. So, it will be appropriate to use the new SpecialType items only if one is not interested in supporting those scenarios. There will be clear comments on the new member(s) regarding the fact.

[CyrusNajmabadi]

SGTM. If the current setup properly encapsulates required semantics, that's good to know.

[CyrusNajmabadi]

Removing from api-review based on Aleksey's explanation for why this is working as intended.

[DoctorKrolic]

With the addition of corresponding SpecialType entries this can be simplified down to a single special type check without the need to request additional type from the compilation.

This is incorrect and this is not a motivation for the compiler to add the API for ReadOnlySpan

I tried to write motivation both for compiler and IDE use. This quote is about how I would use it in IDE/generators scenarios. This doesn't repeal the fact, that compiler needs it for different purposes with different reasoning. I just want to show both sides of the question. After all, if this type was only for compiler use it wouldn't be public, right?

[DoctorKrolic]

IDE usage is also why I included regular Span to the proposal as well. Even in the usage examples these 2 types come close to one another, so having Span in addition to ReadOnlySpan was mostly motivated by the potential IDE needs

[AlekseyTs]

After all, if this type was only for compiler use it wouldn't be public, right?

This doesn't feel like the right way to approach the problem. Certainly any consumer is free to use a public API and benefit from it. However, usage of any API is appropriate only under specific conditions. The usage that was "advertised" as appropriate for IDE features is actually inappropriate. That doesn't mean IDE cannot use the API, it can, but only when it would be in line with the goal. The "advertised" usage is not in line with IDE goals.

[AlekseyTs]

SpecialType entries are not supposed to provide a way of locating "official" definitions for types. Each type has its own rules for locating its "official" definition. For some types the rules are simple, for others they are complicated. SpecialTypes provide a way of locating type definitions in core library and quickly checking if a type is a specific definition from core library. That is all. It doesn't provide anything else. For some types, that is sufficient to locate the "official" definition. For other types it is not. Span types are the second kind. It is that simple. An addition of the entry doesn't change the rules for locating type's "official" definition.

[DoctorKrolic]

@AlekseyTs Updated issue based on your feedback. Span got removed from the proposal, ease of misuse is now a risk. We are not in a hurry, but having a ready-to-go proposal definitely doesn't hurt. Please review

[AlekseyTs]

Looks ready for review.

[333fred]

API Review

• Potential for confusion around being in SpecialType when ROS doesn't come from corlib
  • API has documentation emphasizing this
  • Definitely some angst in the team about this
• Could we make a SpecialTypeInternal?
  • Would need to intercept a public request for the internal constant for ReadOnlySpan to avoid returning it
  • Would allow the compiler to make changes like this that shouldn't be public in the future.

Conclusion: Needs work. Let's investigate a SpecialTypeInternal API instead of adding to SpecialType