Move EditorBrowsableAttribute to Private.CorLib

[karelz]

We have multiple cases where we need to hide methods in Private.CorLib from IntelliSense:

• Deconstruct method on KeyValuePair<TKey,TValue> and DictionaryEntry - Proposal: Supporting C# 7 deconstruction of certain types. #19397
  • We can work around by shipping the method as Extension method
• Span<T> methods Equals and GetHashCode - Span<T> api update coreclr#8583 (comment)
  • No workaround

There is a risk that we won't be able to move the Attribute to mscorlib on Desktop (similar to fallout from moving ExtensionAttribute down - see https://github.com/dotnet/corefx/issues/13746#issuecomment-265829858)
Desktop options:

1. Deal with the impact of the type being moved in Desktop (compat fallout).
2. Don't attribute the methods on Desktop p i.e. they will be visible to IntelliSense on Desktop.
   • Question for @weshaggard: The attribute matters only for ref. What if the attribute is there in .NET Standard and in Desktop ref, but left out in Desktop implementation? Is that an option?
3. Don't port these methods to Desktop - i.e. they can't be part of .NET Standard.
4. Provide the methods as extension methods on Desktop - i.e. they can't be part of .NET Standard.

[weshaggard]

Question for @weshaggard: The attribute matters only for ref. What if the attribute is there in .NET Standard and in Desktop ref, but left out in Desktop implementation? Is that an option?

These attributes are only needed in the reference assemblies and for .NET Core those are independently authored so we could just put them in the ref because EditerBrowsable lives in System.Runtime which is the lowest level ref. For desktop however they are not independently created so it would still need to be moved to mscorlib if we wanted to even use them for types in mscorlib's reference assembly.

[jkotas]

IMHO, there is not much to review from API addition point of view. Whether a type lives in CoreLib or System.Runtime is internal implementation detail in .NET Core...

[karelz]

@jkotas I'd like to discuss the impact on Desktop and our options ... I agree it is not quite API review :)

[AlexGhiondea]

@karelz is issue tracking the work needed for Desktop? Because on Core, as @weshaggard mentioned, that attribute is exposed via the System.Runtime contract.

[karelz]

It's tracking 2 things:

1. Discussion in API review that this is the pattern we want and are ok with.
2. Add the attribute on the particular APIs in .NET Core.

[jkotas]

This came up again during the review of dotnet/coreclr#8695.

I have submitted PR to move it: dotnet/coreclr#8703

[jamesqo]

Should this be closed?

[jkotas]

Not yet - there is follow up corefx change that needs to be done.

[karelz]

Don't we need any changes in CoreFX?

[karelz]

@jkotas can you remind me where we are with this issue?

[jkotas]

This is fixed now.