search

perlang-org / perlang

The Perlang Programming Language
https://perlang.org
MIT License
16 stars 1 forks source link

Enable trimmer analysis warnings #348

Closed perlun closed 4 weeks ago

perlun commented 1 year ago

Moved to GitLab

Please continue to the new version of this issue here: https://gitlab.perlang.org/perlang/perlang/-/issues/348. The GitHub page you are currently reading will not contain the latest information on this issue.

At the moment, we suppress trimmer warnings via SuppressTrimAnalysisWarnings set here: https://github.com/perlang-org/perlang/blob/88f0c8935062e56d5e1a31342067ae05b3d5f8b9/Directory.Build.props#L3-L7

However, suppressing the trimmer warnings is not really the way to go. When we upgraded to .NET 6, we reasoned like this

Suppressing trimmer analysis warnings might seem a bit foolish, but I'm not sure littering the code with tons of attributes to suppress them is really the way forward for us. Let's for now assume the "dynamic programming language" approach instead - let's catch errors with unit tests instead of static analysis. zany_face

Time will tell whether this is insane or simply a stroke of genius...

This hit us when trying to upgrade to .NET 7 (https://github.com/perlang-org/perlang/pull/347#issuecomment-1267420299), since it hid these trimming warnings. This together with the changes in .NET 7 (https://devblogs.microsoft.com/dotnet/announcing-dotnet-7-preview-7/#trimming-and-nativeaot-all-assemblies-trimmed-by-default) turned out to be disastrous.

It was also hard to fully resolve this at the time of the .NET 6 upgrade, because it essentially requires (I think) all dependencies to also properly be trimmer-annotated, and this is not really something fully under our control. This used to be a problem with System.CommandLine, but I'm not sure this is the case any more since https://github.com/dotnet/command-line-api/issues/1465 has been resolved.

Suggested steps

  1. Enable the trimmer warnings and see how horrible the output is.
  2. Upgrade System.CommandLine to a more recent version if it might help.
  3. Annotate the code (or exempt certain classes/assemblies, if possible) with trimmer annotations to make the compilation emit zero trimmer warnings.
perlun commented 1 year ago

Here are the current warnings as of 88f0c8935062e56d5e1a31342067ae05b3d5f8b9:

/home/per/git/perlang/src/Perlang.Interpreter/PerlangInterpreter.cs(122,34): Trim analysis error IL2026: Perlang.Interpreter.PerlangInterpreter.<>c.<RegisterGlobalClasses>b__16_0(Assembly): Using member 'System.Reflection.Assembly.GetTypes()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Types might be removed. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/git/perlang/src/Perlang.Interpreter/Typing/TypeResolver.cs(500,17): Trim analysis error IL2075: Perlang.Interpreter.Typing.TypeResolver.VisitGetExpr(Expr.Get): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethods()'. The return value of method 'Perlang.ITypeReference.ClrType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
ILLink : Trim analysis error IL2026: System.ComponentModel.Design.DesigntimeLicenseContextSerializer.DeserializeUsingBinaryFormatter(DesigntimeLicenseContextSerializer.StreamWrapper, String, RuntimeLicenseContext): Using member 'System.Runtime.Serialization.IFormatter.Deserialize(Stream)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. BinaryFormatter serialization is not trim compatible because the Type of objects being processed cannot be statically discovered. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
ILLink : Trim analysis error IL2026: System.ComponentModel.Design.DesigntimeLicenseContextSerializer.SerializeWithBinaryFormatter(Stream, String, DesigntimeLicenseContext): Using member 'System.Runtime.Serialization.IFormatter.Serialize(Stream, Object)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. BinaryFormatter serialization is not trim compatible because the Type of objects being processed cannot be statically discovered. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/.nuget/packages/microsoft.netcore.app.runtime.linux-x64/7.0.0-rc.1.22426.10/runtimes/linux-x64/lib/net7.0/System.Runtime.InteropServices.dll : error IL2104: Assembly 'System.Runtime.InteropServices' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/.nuget/packages/system.commandline/2.0.0-beta1.21216.1/lib/netstandard2.0/System.CommandLine.dll : error IL2104: Assembly 'System.CommandLine' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/usr/share/dotnet/sdk/7.0.100-rc.1.22431.12/Sdks/Microsoft.NET.ILLink.Tasks/build/Microsoft.NET.ILLink.targets(86,5): error NETSDK1144: Optimizing assemblies for size failed. Optimization can be disabled by setting the PublishTrimmed property to false. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
perlun commented 1 year ago

Hmm... with #350 in place, we get rid of that particular System.CommandLine warning but I get the feeling that there's still some issue there. Applying this diff:

commit ac5f898f6d0deb64be91f76eb12bfa032f222478
Author: Per Lundberg <perlun@gmail.com>
Date:   Thu Oct 20 20:56:30 2022 +0300

    (many) Enable trimmer warnings

diff --git Directory.Build.props Directory.Build.props
index 2ecb570..44182a1 100644
--- Directory.Build.props
+++ Directory.Build.props
@@ -3,7 +3,7 @@
     <PropertyGroup>
         <CodeAnalysisRuleSet>$(SolutionDir)global.ruleset</CodeAnalysisRuleSet>
         <TargetFramework>net7.0</TargetFramework>
-        <SuppressTrimAnalysisWarnings>true</SuppressTrimAnalysisWarnings>
+        <SuppressTrimAnalysisWarnings>false</SuppressTrimAnalysisWarnings>
     </PropertyGroup>

     <PropertyGroup Condition="$([MSBuild]::IsOSPlatform('Linux'))">
diff --git src/Perlang.Common/ITypeReference.cs src/Perlang.Common/ITypeReference.cs
index 2d9669a..8553eb1 100644
--- src/Perlang.Common/ITypeReference.cs
+++ src/Perlang.Common/ITypeReference.cs
@@ -1,6 +1,7 @@
 #nullable enable

 using System;
+using System.Diagnostics.CodeAnalysis;

 namespace Perlang
 {
@@ -9,6 +10,7 @@ namespace Perlang
         Token? TypeSpecifier { get; }

         // TODO: Remove setter to make this interface and class be fully immutable, for debuggability.
+        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
         Type? ClrType { get; set; }

         /// <summary>
diff --git src/Perlang.Common/TypeReference.cs src/Perlang.Common/TypeReference.cs
index 349564d..70d9135 100644
--- src/Perlang.Common/TypeReference.cs
+++ src/Perlang.Common/TypeReference.cs
@@ -1,5 +1,6 @@
 #nullable enable
 using System;
+using System.Diagnostics.CodeAnalysis;

 namespace Perlang
 {
@@ -15,6 +16,7 @@ namespace Perlang

         public Type? ClrType
         {
+            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
             get => clrType;
             set
             {
diff --git src/Perlang.Interpreter/PerlangInterpreter.cs src/Perlang.Interpreter/PerlangInterpreter.cs
index e087512..8c0a3dc 100644
--- src/Perlang.Interpreter/PerlangInterpreter.cs
+++ src/Perlang.Interpreter/PerlangInterpreter.cs
@@ -3,6 +3,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Linq;
 using System.Numerics;
@@ -116,6 +117,7 @@ namespace Perlang.Interpreter
         /// Registers global classes defined in native .NET code.
         /// </summary>
         /// <exception cref="PerlangInterpreterException">Multiple classes with the same name was encountered.</exception>
+        [UnconditionalSuppressMessage("AssemblyLoadTrimming", "IL2026:RequiresUnreferencedCode", Justification = "Everything referenced in the loaded assembly is manually preserved, so it's safe")]
         private void RegisterGlobalClasses()
         {
             var globalClassesQueryable = AppDomain.CurrentDomain.GetAssemblies()
diff --git src/Perlang.Interpreter/Typing/TypeResolver.cs src/Perlang.Interpreter/Typing/TypeResolver.cs
index fccdc6b..104320f 100644
--- src/Perlang.Interpreter/Typing/TypeResolver.cs
+++ src/Perlang.Interpreter/Typing/TypeResolver.cs
@@ -2,6 +2,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Numerics;
 using Humanizer;
@@ -457,6 +458,7 @@ namespace Perlang.Interpreter.Typing
             return VoidObject.Void;
         }

+        [UnconditionalSuppressMessage("AssemblyLoadTrimming", "IL2026:RequiresUnreferencedCode", Justification = "Everything referenced in the loaded assembly is manually preserved, so it's safe")]
         public override VoidObject VisitGetExpr(Expr.Get expr)
         {
             base.VisitGetExpr(expr);

...causes the following link-time warnings:

/home/per/git/perlang/src/Perlang.Common/TypeReference.cs(20,20): Trim analysis error IL2041: Perlang.TypeReference.ClrType.get: The 'DynamicallyAccessedMembersAttribute' is not allowed on methods. It is allowed on method return value or method parameters though. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/git/perlang/src/Perlang.Common/TypeReference.cs(20,20): Trim analysis error IL2093: Perlang.TypeReference.ClrType.get: 'DynamicallyAccessedMemberTypes' in 'DynamicallyAccessedMembersAttribute' on the return value of method 'Perlang.TypeReference.ClrType.get' don't match overridden return value of method 'Perlang.ITypeReference.ClrType.get'. All overridden members must have the same 'DynamicallyAccessedMembersAttribute' usage. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/git/perlang/src/Perlang.Common/TypeReference.cs(23,17): Trim analysis error IL2092: Perlang.TypeReference.ClrType.set: 'DynamicallyAccessedMemberTypes' in 'DynamicallyAccessedMembersAttribute' on the parameter 'value' of method 'Perlang.TypeReference.ClrType.set' don't match overridden parameter 'value' of method 'Perlang.ITypeReference.ClrType.set'. All overridden members must have the same 'DynamicallyAccessedMembersAttribute' usage. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/git/perlang/src/Perlang.Interpreter/Typing/TypeResolver.cs(521,17): Trim analysis error IL2072: Perlang.Interpreter.Typing.TypeResolver.VisitGetExpr(Expr.Get): 'value' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'Perlang.ITypeReference.ClrType.set'. The return value of method 'System.Reflection.MethodInfo.ReturnType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/git/perlang/src/Perlang.Interpreter/Typing/TypeResolver.cs(393,21): Trim analysis error IL2072: Perlang.Interpreter.Typing.TypeResolver.VisitLiteralExpr(Expr.Literal): 'value' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'Perlang.ITypeReference.ClrType.set'. The return value of method 'System.Object.GetType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/git/perlang/src/Perlang.Interpreter/Typing/TypeResolver.cs(397,21): Trim analysis error IL2072: Perlang.Interpreter.Typing.TypeResolver.VisitLiteralExpr(Expr.Literal): 'value' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'Perlang.ITypeReference.ClrType.set'. The return value of method 'System.Object.GetType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
ILLink : Trim analysis error IL2026: System.ComponentModel.Design.DesigntimeLicenseContextSerializer.DeserializeUsingBinaryFormatter(DesigntimeLicenseContextSerializer.StreamWrapper, String, RuntimeLicenseContext): Using member 'System.Runtime.Serialization.IFormatter.Deserialize(Stream)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. BinaryFormatter serialization is not trim compatible because the Type of objects being processed cannot be statically discovered. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
ILLink : Trim analysis error IL2026: System.ComponentModel.Design.DesigntimeLicenseContextSerializer.SerializeWithBinaryFormatter(Stream, String, DesigntimeLicenseContext): Using member 'System.Runtime.Serialization.IFormatter.Serialize(Stream, Object)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. BinaryFormatter serialization is not trim compatible because the Type of objects being processed cannot be statically discovered. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/.nuget/packages/microsoft.netcore.app.runtime.linux-x64/7.0.0-rc.1.22426.10/runtimes/linux-x64/lib/net7.0/System.Runtime.InteropServices.dll : error IL2104: Assembly 'System.Runtime.InteropServices' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/home/per/.nuget/packages/system.commandline.namingconventionbinder/2.0.0-beta4.22272.1/lib/netstandard2.0/System.CommandLine.NamingConventionBinder.dll : error IL2104: Assembly 'System.CommandLine.NamingConventionBinder' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
/usr/share/dotnet/sdk/7.0.100-rc.1.22431.12/Sdks/Microsoft.NET.ILLink.Tasks/build/Microsoft.NET.ILLink.targets(86,5): error NETSDK1144: Optimizing assemblies for size failed. Optimization can be disabled by setting the PublishTrimmed property to false. [/home/per/git/perlang/src/Perlang.ConsoleApp/Perlang.ConsoleApp.csproj]
perlun commented 1 year ago

Reported the System.CommandLine.NamingConventionBinder issue upstream: https://github.com/dotnet/command-line-api/issues/1775#issuecomment-1285977150

perlun commented 1 year ago

I think we're still not ready for this. It seems nontrivial to get this done, especially in that it's hard to annotate property getters only (and not setters). As mentioned in the issue description, I have changed my mind since https://github.com/perlang-org/perlang/pull/223#discussion_r747787950 but let's postpone it until next year or so. :slightly_smiling_face: