Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InlineArray] should not have [EditorBrowsableState(Never)] #94283

Closed
Sergio0694 opened this issue Nov 2, 2023 · 6 comments · Fixed by #94308
Closed

[InlineArray] should not have [EditorBrowsableState(Never)] #94283

Sergio0694 opened this issue Nov 2, 2023 · 6 comments · Fixed by #94308
Assignees
Labels
area-System.Runtime.CompilerServices help wanted [up-for-grabs] Good issue for external contributors

Comments

@Sergio0694
Copy link
Contributor

Sergio0694 commented Nov 2, 2023

Description

The [InlineArray] type is marked with [EditorBrowsable(EditorBrowsableState.Never)]:

[EditorBrowsable(EditorBrowsableState.Never)]
[AttributeUsage(AttributeTargets.Struct, AllowMultiple = false)]
public sealed class InlineArrayAttribute : Attribute

This seems... Wrong? There's no built-in C# syntax for inline arrays for now, so the attribute has to be used explicitly in user code. The fact it's hidden makes this clunkier than it needs to be. The attribute is already buried in a namespace that's explicitly documented as "unsafe" and only for advanced users, so this feels unnecessary.

Shouldn't we just remove this?
And if so, I take it the ship has sailed for .NET 8 GA at this point?

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 2, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 2, 2023
@vcsjones vcsjones added area-System.Runtime.CompilerServices and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 2, 2023
@ghost
Copy link

ghost commented Nov 2, 2023

Tagging subscribers to this area: @dotnet/area-system-runtime-compilerservices
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

The [InlineArray] type is marked with [EditorBrowsable(EditorBrowsableState.Never)]:

[EditorBrowsable(EditorBrowsableState.Never)]
[AttributeUsage(AttributeTargets.Struct, AllowMultiple = false)]
public sealed class InlineArrayAttribute : Attribute

This seems... Wrong? There's no built-in C# syntax for inline arrays for now, so the attribute has to be used explicitly in user code. The fact it's hidden makes this clunkier than it needs to be. The attribute is already buried in a namespace that's explicitly documented as "unsafe" and only for advanced users, so this feels unnecessary.

Shouldn't we just remove this?
And if so, I take it the ship has sailed for .NET 8 GA at this point?

Author: Sergio0694
Assignees: -
Labels:

area-System.Runtime.CompilerServices, untriaged

Milestone: -

@MichalStrehovsky
Copy link
Member

Cc @VSadov - I can't find where the EditorBrowsable came from. I don't see the API review mentioning this.

@stephentoub
Copy link
Member

Shouldn't we just remove this?

Yes, this should not be EditorBrowsableState.Never. We should remove it, and ideally service that for 8.

@Sergio0694
Copy link
Contributor Author

Happy to take this and make a PR if this is up for grabs. I can also add the missing XML docs while at it 🙂

@tannergooding tannergooding added help wanted [up-for-grabs] Good issue for external contributors and removed untriaged New issue has not been triaged by the area owner labels Nov 2, 2023
@tannergooding
Copy link
Member

@Sergio0694, assigned to you

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 2, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 3, 2023
@VSadov
Copy link
Member

VSadov commented Nov 5, 2023

I think the attribute was simply copied from other similar types when we still had assumptions that users will not need to put the attribute directly in the code, but instead compilers would emit it when implementing inline-array-like features.
In the final design of the feature on C# side there is indeed no special syntax to declare inline array types and thus it requires using the attribute.

@Sergio0694 Thanks for fixing this!!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.CompilerServices help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants