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.

Sign up for GitHub

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

Jump to bottom

Move ILLink.LinkAttributes to shared and support NullabilityInfoContext with trimming nullability attributes #56475

Merged
merged 7 commits into from
Jul 30, 2021
Merged

Move ILLink.LinkAttributes to shared and support NullabilityInfoContext with trimming nullability attributes #56475

merged 7 commits into from
Jul 30, 2021

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Jul 28, 2021 •
edited
Loading

This addresses how we are trimming attributes in .NET 6. The problems being solved here are:

  1. A lot of attributes are only trimmed on mono, and not coreclr. Merging the list to be shared, where applicable.
  2. Add a feature switch to turn on/off trimming nullability attributes. When this switch is off, NullabilityInfoContext.Create will throw an exception.
  3. Adding a feature switch to allow for app developers to specify whether they want the aggressive attribute trimming. This allows for Blazor WASM and Maui apps to aggressively trim these attributes, while other app models don't. App developers can flip it either way, giving them the power to decide.

Note: It is probably easiest to review commit-by-commit. The 1st commit is a straight copy from mono to shared, while changes are made in the 2nd commit.

Fix #48217
Fix #55860

@jkotas @MichalStrehovsky @davidwrighton - are there other attributes besides IntrinsicAttribute in this list that shouldn't be removed in coreclr? For example, questionable ones for me were: NonVersionableAttribute, AsyncMethodBuilderAttribute, SkipLocalsInitAttribute, EmbeddedAttribute, NativeIntegerAttribute, or IsUnmanagedAttribute?

runtime/src/mono/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.xml

Lines 4 to 191 in 6789379

<type fullname="System.CLSCompliantAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.ObsoleteAttribute">
<!--
Note that removing this attribute can change runtime behavior. For example,
System.Xml.Serialization will behave differently if a ctor is Obsolete.
This is low enough risk on wasm to justify the removing the attribute for size
savings. The app developer can override this setting to keep all ObsoleteAttributes.
-->
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Diagnostics.CodeAnalysis -->
<type fullname="System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Reflection.AssemblyCompanyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyConfigurationAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyCopyrightAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyDefaultAliasAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyDescriptionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyProductAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyTitleAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.AsyncMethodBuilderAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerArgumentExpressionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerFilePathAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerLineNumberAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CompilerGlobalScopeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.IsReadOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.EnumeratorCancellationAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.ExtensionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.IntrinsicAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.SkipLocalsInitAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.TupleElementNamesAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.Versioning -->
<type fullname="System.Runtime.Versioning.SupportedOSPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.UnsupportedOSPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.SupportedOSPlatformGuardAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.UnsupportedOSPlatformGuardAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.TargetPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.ComponentModel -->
<type fullname="System.ComponentModel.EditorBrowsableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- Internal attributes shared as implementation, so they could be in any assembly -->
<assembly fullname="*">
<type fullname="System.Runtime.Versioning.NonVersionableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- Attributes that are allowed to be in any assembly -->
<assembly fullname="*">
<type fullname="System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.RequiresAssemblyFilesAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.SuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.AllowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DisallowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnIfAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- The following attributes are generated by the compiler, so they could be in any assembly -->
<assembly fullname="*">
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.IsUnmanagedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NativeIntegerAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableContextAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullablePublicOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- Microsoft.CodeAnalysis -->
<type fullname="Microsoft.CodeAnalysis.EmbeddedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>

@eerhardt
Copy ILLink.LinkAttributes from mono to shared.
7f302c8
@eerhardt
Update ILLink.LinkAttributes that are now shared between mono and cor…
ddd9f6d 
…eclr

- Move IntrinsicAttribute to mono only
- Introduce feature switches for NullabilityInfoContext and AggressiveAttributeTrimming
- Minor other cleanup
@eerhardt
Respect the NullabilityInfoContext.IsSupported feature switch in Null…
f5826dd 
…abilityInfoContext.
@eerhardt
Document new feature switches
e1cabb5
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@eerhardt eerhardt added area-System.Reflection linkable-framework Issues associated with delivering a linker friendly framework and removed area-CoreLib-mono new-api-needs-documentation labels Jul 28, 2021
@ghost
Copy link

ghost commented Jul 28, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

This addresses how we are trimming attributes in .NET 6. The problems being solved here are:

  1. A lot of attributes are only trimmed on mono, and not coreclr. Merging the list to be shared, where applicable.
  2. Add a feature switch to turn on/off trimming nullability attributes. When this switch is off, NullabilityInfoContext.Create will throw an exception.
  3. Adding a feature switch to allow for app developers to specify whether they want the aggressive attribute trimming. This allows for Blazor WASM and Maui apps to aggressively trim these attributes, while other app models don't. App developers can flip it either way, giving them the power to decide.

Note: It is probably easiest to review commit-by-commit. The 1st commit is a straight copy from mono to shared, while changes are made in the 2nd commit.

Fix #48217
Fix #55860

@jkotas @MichalStrehovsky @davidwrighton - are there other attributes besides IntrinsicAttribute in this list that shouldn't be removed in coreclr?

runtime/src/mono/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.xml

Lines 4 to 191 in 6789379

<type fullname="System.CLSCompliantAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.ObsoleteAttribute">
<!--
Note that removing this attribute can change runtime behavior. For example,
System.Xml.Serialization will behave differently if a ctor is Obsolete.
This is low enough risk on wasm to justify the removing the attribute for size
savings. The app developer can override this setting to keep all ObsoleteAttributes.
-->
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Diagnostics.CodeAnalysis -->
<type fullname="System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Reflection.AssemblyCompanyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyConfigurationAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyCopyrightAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyDefaultAliasAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyDescriptionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyProductAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyTitleAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.AsyncMethodBuilderAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerArgumentExpressionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerFilePathAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerLineNumberAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CompilerGlobalScopeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.IsReadOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.EnumeratorCancellationAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.ExtensionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.IntrinsicAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.SkipLocalsInitAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.TupleElementNamesAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.Versioning -->
<type fullname="System.Runtime.Versioning.SupportedOSPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.UnsupportedOSPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.SupportedOSPlatformGuardAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.UnsupportedOSPlatformGuardAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.TargetPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.ComponentModel -->
<type fullname="System.ComponentModel.EditorBrowsableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- Internal attributes shared as implementation, so they could be in any assembly -->
<assembly fullname="*">
<type fullname="System.Runtime.Versioning.NonVersionableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- Attributes that are allowed to be in any assembly -->
<assembly fullname="*">
<type fullname="System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.RequiresAssemblyFilesAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.SuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.AllowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DisallowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnIfAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- The following attributes are generated by the compiler, so they could be in any assembly -->
<assembly fullname="*">
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.IsUnmanagedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NativeIntegerAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableContextAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullablePublicOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- Microsoft.CodeAnalysis -->
<type fullname="Microsoft.CodeAnalysis.EmbeddedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>

Author: eerhardt
Assignees: -
Labels:

area-System.Reflection, linkable-framework
Milestone: -

jkotas
jkotas reviewed Jul 28, 2021
View reviewed changes
docs/workflow/trimming/feature-switches.md
@@ -25,6 +25,8 @@ configurations but their defaults might vary as any SDK can set the defaults dif
| EnableCppCLIHostActivation | System.Runtime.InteropServices.EnableCppCLIHostActivation | C++/CLI host activation code is disabled when set to false and related functionality can be trimmed. |
| MetadataUpdaterSupport | System.Reflection.Metadata.MetadataUpdater.IsSupported | Metadata update related code to be trimmed when set to false |
| _EnableConsumingManagedCodeFromNativeHosting | System.Runtime.InteropServices.EnableConsumingManagedCodeFromNativeHosting | Getting a managed function from native hosting is disabled when set to false and related functionality can be trimmed. |
| NullabilityInfoContextSupport | System.Reflection.NullabilityInfoContext.IsSupported | Nullable attributes can be trimmed when set to false |
| _AggressiveAttributeTrimming | System.AggressiveAttributeTrimming | When set to true, aggressively trims attributes to allow for the most size savings possible, even if it could result in runtime behavior changes |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the name have the _ prefix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same reason _EnableConsumingManagedCodeFromNativeHosting has a _ prefix. This isn't intended to be a "public" setting, but only exists for Blazor WASM to trim these attributes, and an escape hatch for developers in case we got it wrong.

buyaa-n
buyaa-n approved these changes Jul 28, 2021
View reviewed changes
Copy link
Contributor

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NullabilityInfoContext related changes LGTM

vitek-karas
vitek-karas approved these changes Jul 28, 2021
View reviewed changes
Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - except for the selection of which attributes are dangerous to trim, I'll leave that to more knowledgeable people.

src/libraries/System.Runtime/tests/System/Reflection/NullabilityInfoContextTests.cs Show resolved Hide resolved
@eerhardt
Copy link
Member Author

Adding a few more coreclr devs to answer this question:

are there other attributes besides IntrinsicAttribute in this list that shouldn't be removed in coreclr? For example, questionable ones for me were: NonVersionableAttribute, AsyncMethodBuilderAttribute, SkipLocalsInitAttribute, EmbeddedAttribute, NativeIntegerAttribute, or IsUnmanagedAttribute?

runtime/src/mono/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.xml

Lines 4 to 191 in 6789379

<type fullname="System.CLSCompliantAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.ObsoleteAttribute">
<!--
Note that removing this attribute can change runtime behavior. For example,
System.Xml.Serialization will behave differently if a ctor is Obsolete.
This is low enough risk on wasm to justify the removing the attribute for size
savings. The app developer can override this setting to keep all ObsoleteAttributes.
-->
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Diagnostics.CodeAnalysis -->
<type fullname="System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Reflection.AssemblyCompanyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyConfigurationAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyCopyrightAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyDefaultAliasAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyDescriptionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyProductAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Reflection.AssemblyTitleAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.AsyncMethodBuilderAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerArgumentExpressionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerFilePathAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerLineNumberAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CallerMemberNameAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.CompilerGlobalScopeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.IsReadOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.EnumeratorCancellationAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.ExtensionAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.IntrinsicAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.SkipLocalsInitAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.TupleElementNamesAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.Runtime.Versioning -->
<type fullname="System.Runtime.Versioning.SupportedOSPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.UnsupportedOSPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.SupportedOSPlatformGuardAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.UnsupportedOSPlatformGuardAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.Versioning.TargetPlatformAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- System.ComponentModel -->
<type fullname="System.ComponentModel.EditorBrowsableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- Internal attributes shared as implementation, so they could be in any assembly -->
<assembly fullname="*">
<type fullname="System.Runtime.Versioning.NonVersionableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- Attributes that are allowed to be in any assembly -->
<assembly fullname="*">
<type fullname="System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.RequiresAssemblyFilesAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.SuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.UnconditionalSuppressMessageAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.AllowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DisallowNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.DoesNotReturnIfAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Diagnostics.CodeAnalysis.MemberNotNullWhenAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>
<!-- The following attributes are generated by the compiler, so they could be in any assembly -->
<assembly fullname="*">
<!-- System.Runtime.CompilerServices -->
<type fullname="System.Runtime.CompilerServices.IsUnmanagedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NativeIntegerAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullableContextAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<type fullname="System.Runtime.CompilerServices.NullablePublicOnlyAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
<!-- Microsoft.CodeAnalysis -->
<type fullname="Microsoft.CodeAnalysis.EmbeddedAttribute">
<attribute internal="RemoveAttributeInstances" />
</type>
</assembly>

cc @JulieLeeMSFT @AndyAyersMS @tommcdon @agocke

It looks like NonVersionableAttribute is inspected by ReadyToRun, so maybe that one should be moved back to just "mono":

runtime/src/coreclr/vm/methodtable.cpp

Lines 9849 to 9863 in 3ce1168

if (!pMT->GetModule()->IsInCurrentVersionBubble())
{
if (!pMT->IsValueType())
{
// Eventually, we may respect the non-versionable attribute for reference types too. For now, we are going
// to play is safe and ignore it.
return FALSE;
}
// Valuetypes with non-versionable attribute are candidates for fixed layout. Reject the rest.
if (pMT->GetModule()->GetMDImport()->GetCustomAttributeByName(pMT->GetCl(),
NONVERSIONABLE_TYPE, NULL, NULL) != S_OK)
{
return FALSE;
}

@AndyAyersMS
Copy link
Member

I would be leery of trimming any custom attribute that the runtime checks for. I don't know how to effectively enumerate that set, perhaps @jkotas or @davidwrighton can say...

@eerhardt
Copy link
Member Author

I would be leery of trimming any custom attribute that the runtime checks for. I don't know how to effectively enumerate that set

Would this be a decent approach?

foreach (Attribute in the above list)
{
   if (coreclr code references the attribute)
   {
      move attribute back to mono-only();
   }
}

@AndyAyersMS
Copy link
Member

Would this be a decent approach?

Maybe? I don't how easy it actually is to enumerate all the attributes. One place to look is https://github.com/dotnet/runtime/blob/main/src/coreclr/vm/wellknownattributes.h but I suspect it's just convention that attributes are listed there and there may be others that the runtime looks for.

@eerhardt
Copy link
Member Author

My thought was just to do a grep for the attribute name in the coreclr folder. If it shows up in a grep, the assumption is that coreclr uses it for something.

@davidwrighton
Copy link
Member

My look at this list found only NonVersionableAttribute and IntrinsicAttribute as problematic. @eerhardt the idea of using grep seems good tome.

@eerhardt
Copy link
Member Author

eerhardt commented Jul 29, 2021 •
edited
Loading

After searching for each attribute in the list in src/coreclr, here's the hits I found:

  • IntrinsicAttribute and NonVersionableAttribute should definitely move to mono-only.
  • NullableAttribute

    runtime/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/AttributePresenceFilterNode.cs

    Lines 53 to 57 in 57bfe47

    // System.Runtime.CompilerServices.NullableAttribute is NEVER added to the table (There are *many* of these, and they provide no useful value to the runtime)
    if (customAttributeTypeNamespace == "System.Runtime.CompilerServices" && customAttributeTypeName == "NullableAttribute")
    {
    continue;
    }

    runtime/src/coreclr/zap/zapreadytorun.cpp

    Lines 520 to 522 in 57bfe47

    // System.Runtime.CompilerServices.NullableAttribute is NEVER added to the table (There are *many* of these, and they provide no useful value to the runtime)
    if ((strcmp(szNamespace, "System.Runtime.CompilerServices") == 0) && (strcmp(szName, "NullableAttribute") == 0))
    continue;
    • These places are just filtering out the NullableAttribute. So if we trim it, it won't be in the list anyway. So this should be fine.
  • TupleElementNamesAttribute is referenced in a code comment here:

    runtime/src/coreclr/ilasm/assembler.cpp

    Lines 2691 to 2699 in 57bfe47

    // We now need to fetch the values of the new GenericParamConstraint tokens
    // that were created by the call to SetGenericParamProps
    //
    // These tokens are the token owners for the custom attributes
    // such as NulableAttribute and TupleElementNamesAttribute
    //
    HCORENUM hEnum = NULL;
    ULONG uCount = 0;
    if (FAILED(m_pImporter->EnumGenericParamConstraints(&hEnum, tkGenericParam, currGPConstraintArr, (ULONG)currNumConstraints, &uCount)))
    • This is in ilasm, so I'm not sure it "counts" as coreclr runtime
  • UnconditionalSuppressMessageAttribute

    runtime/src/coreclr/tools/Common/Compiler/Logger.cs

    Lines 78 to 91 in 57bfe47

    internal bool IsWarningSuppressed(int code, MessageOrigin origin)
    {
    if (_suppressedWarnings.Contains(code))
    return true;
    IEnumerable<CustomAttributeValue<TypeDesc>> suppressions = null;
    // TODO: Suppressions with different scopes
    if (origin.MemberDefinition is MethodDesc method)
    {
    var ecmaMethod = method.GetTypicalMethodDefinition() as EcmaMethod;
    suppressions = ecmaMethod?.GetDecodedCustomAttributes("System.Diagnostics.CodeAnalysis", "UnconditionalSuppressMessageAttribute");
    • I think this is NativeAOT code, which the trimmer doesn't run before the NativeAOT compiler, so this should be fine. cc @MichalStrehovsky
  • AssemblyMetadataAttribute

    runtime/src/coreclr/inc/corhdr.h

    Lines 1875 to 1876 in 57bfe47

    #define ASSEMBLY_METADATA_TYPE_W W("System.Reflection.AssemblyMetadataAttribute")
    #define ASSEMBLY_METADATA_TYPE "System.Reflection.AssemblyMetadataAttribute"
    • This appears to be an unused define. I'll remove it in a separate PR

@eerhardt eerhardt mentioned this pull request Jul 30, 2021
Merged
@eerhardt eerhardt merged commit 577d079 into dotnet:main Jul 30, 2021
@eerhardt eerhardt deleted the ILLinkAttributes branch July 30, 2021 19:07
eerhardt added a commit to dotnet/sdk that referenced this pull request Jul 31, 2021
@eerhardt
Update SDK to support new DI and Attribute feature switches (#19432)
e10860e 
See
* dotnet/runtime#55102
* dotnet/runtime#56475
@jaredpar jaredpar mentioned this pull request Aug 17, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Aug 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jkotas jkotas jkotas left review comments

@buyaa-n buyaa-n buyaa-n approved these changes

@vitek-karas vitek-karas vitek-karas approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar

@sbomer sbomer Awaiting requested review from sbomer

@MichalStrehovsky MichalStrehovsky Awaiting requested review from MichalStrehovsky

Assignees
No one assigned
Labels
area-System.Reflection linkable-framework Issues associated with delivering a linker friendly framework
Projects
No open projects
Triage POD for Reflection, META, etc.
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NullabilityInfoContext + Trimming Move ILLink.LinkAttributes.xml to a shared location
6 participants
@eerhardt @AndyAyersMS @davidwrighton @jkotas @vitek-karas @buyaa-n