Add System.Runtime.CompilerServices.ReadOnlyAttribute#10777
Add System.Runtime.CompilerServices.ReadOnlyAttribute#10777jkotas merged 1 commit intodotnet:masterfrom OmarTawfik:System.Runtime.CompilerServices.ReadOnlyAttribute-proposal
Conversation
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
|
|
There was a problem hiding this comment.
Convention of other files in the same folder :( #Closed
|
|
||
| namespace System.Runtime.CompilerServices | ||
| { | ||
| [AttributeUsage(AttributeTargets.All, Inherited = false)] |
There was a problem hiding this comment.
Should add doc comment detailing this is an attribute used by the compiler to trakc additional metadata. Should not be used by developers manually. #Resolved
There was a problem hiding this comment.
Maybe mark type with EditorBrowsableAttribute(EditorBrowsableState.Never)?
On the other hand, I think the entire CompilerServices namespace is already meant for compiler consumption (but not general consumption), so may be superfluous. #Resolved
There was a problem hiding this comment.
I think the entire CompilerServices namespace is already meant for compiler consumption (but not general consumption), so may be superfluous
There are types in the compiler namespace that folks use, though, like ConditionalWeakTable, StrongBox, even AsyncTaskMethodBuilder. I think adding the EditorBrowsable attribute would be reasonable. #Resolved
There was a problem hiding this comment.
My plan for now is to produce errors when users use it in their source code, same as we do for Dynamic attribute anyways. Will add the comment. #Closed
jkotas
left a comment
There was a problem hiding this comment.
Could you please add this under src\mscorlib\shared so that it is added for both CoreCLR and CoreRT?
|
@terrajobst @karelz do I need sign off from API before merging in? |
|
The API issues is marked as approved. There is no additional sign off required (except regular PR review). |
Proposal is in: https://github.com/dotnet/corefx/issues/17741
Questions for area owners:
model.xmlis no longer used in the repo.Is there any additional work needed to copy this to corert repo? or is the mirroring tool going to handle it?cc @VSadov @jcouv @dotnet/roslyn-compiler