Compiler Trim Warnings

[rockfordlhotka]

Discussed in #4424

^{Originally posted by Bowman74 January 1, 2025}
@rockfordlhotka

Currently we have suppressed many of the trim warnings in CSLA. The following are suppressed:
IL2026 IL2055 IL2057 IL2060 IL2070 IL2072 IL2075 IL21111 IL3050

I looked into what it would take to resolved them. Many of them cannot be fixed without re-thinking interfaces like IDataPortalServer's Update method's obj parameter. Right now the parameter is System.object, something outside of our control.

The problem is in DataPortal.cs (Csla.Server.DataPortal) that implements IDataPortalServer. It the implementation of the Update method it calls into Csla.Reflection.ServiceProviderMethodCaller.FindDataPortalMethod passing in an object Type to the targettype parameter. That parameter in the FindDataPortalMethod method is decorated with:

[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]

Wherever that type comes from also has to be decorated. If we look back at the Update method we find this line where the type is gotten:

objectType = obj.GetType();

OK, so in order to clear this warning we need to make sure that obj, whatever it is, is similarly decorated. So we look back and find that obj is the Update method's parameter of type System.object. We can't decorate that.

Three ways to fix come to mind. All three of these require IBusinessObject and all derived types to be annotated with the DynamicallyAccessedMembers if on .Net 8+. This will require user code for business objects to also have annotations if they don't want the same warnings, which makes that they would have to tell the compiler not to get rid of all those apparently dead wood data portal methods.

Here are my ideas on ways to fix:

• (Probably not) Change the parameter type of the IDataPortalServer's Update method to IBusinessObject from System.Object. Breaking change for anyone who created a custom implementation of any of these interfaces, ugh...
• (Breaking update for .Net version 8+ only, worse for maintenance) Change the IDataPortalServer's interface to use IBusinessObject on .Net 8+, all lower versions still use System.Object. More backward compatible, only causes a potential issue when revving .Net version to 8+. Ugly code with lots more #ifs.
• Leave the IDataPortalServer interface alone so parameter is still System.Object. In the Update method if .Net 8+ we check to see if the obj parameter is castable to IBusinessObject. It not we throw some sort of invalid parameter exception. If it is of type IBusinessObject we cast it to that type and call GetType on that, clearing the warning.

There are other possible solutions like using [DynamicDependency]. Microsoft does not recommend this but it is a possibility.

https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/prepare-libraries-for-trimming

[rockfordlhotka]

The reason that the data portal works against type object is that technically the data portal has always been able to work against any type of serializable object even if it doesn't subclass a CSLA base type.

I find it hard to imagine anyone actually does serialize arbitrary types anymore. Now that the only serializable types must implement IMobileObject, that has become the most basic common denominator from a type perspective.

However, I am not sure it would be a bad thing to do as you suggest and require IBusinessObject to be the most basic type. Or to create a yet more basic interface, because these days IBusinessObject actually has a member: Identity.

At least this basic change to the data portal interface needs to occur for CSLA 9.0, because (as you say) it is a breaking change).

I am kind of thinking we should do this:

ICslaObject <- IBusinessObject <- other base interfaces

And make the data portal require ICslaObject - where ICslaObject has no members and so provides the lowest possible barrier for implementing future types that might flow through the data portal.

[Bowman74]

Yes, instead of IBusinessObject we could make a new simpler type that is annotated like ICslaObject object or something. Then we could have IBusinessObject inherit from that. Then ICslaObject would be the parameter type for methods Like IDataPortalServer's Update method.

That would force the annotations into user code but that's what would be required for trimming. I would guess the goal would be to make CSLA and CSLA.Xaml at least trim friendly with the proper annotations to ensure that the compiler keeps what needs to be kept.

I believe we no longer use incompatible serializers like the BinarySerializer. But to be sure we should compare to the known incompatibilities list to make sure we are not running afoul of any of them. In my brief perusal I think we are good, at least for the CSLA core library and CSLA.Xaml.

https://learn.microsoft.com/en-us/dotnet/core/deploying/trimming/incompatibilities

[Bowman74]

We may also want to explore the RequiresUnreferencedCodeAttribute for the dataportal related reflection. This would basically mean that the dataportal, and anything passed into it by extension, is fundamentally not trimmable.

[rockfordlhotka]

I'd like @kant2002 to give any insights too.

[rockfordlhotka]

That would force the annotations into user code but that's what would be required for trimming. I would guess the goal would be to make CSLA and CSLA.Xaml at least trim friendly with the proper annotations to ensure that the compiler keeps what needs to be kept.

I would prefer to not push this into user code. On the other hand, perhaps that's unavoidable, and the future of .NET is code full of attributes to deal with AOT and the linker.

I also wonder if some of this might become less important if we implement #4359 with a generator to make all the data portal calls (at least appear) strongly typed. That issue wouldn't affect dynamic creation of instances based on a type, but would/will ensure that the data portal methods in a business class are invoked in a way that the compiler knows they are being invoked.

[kant2002]

@rockfordlhotka I state multiple times and will repeat again. I'm strongly against using DynamicallyAccessedMembers(All) at this stage. If there some limited things like DynamicallyAccessedMembers(publicConstructors) that maybe fine. The less we annotate with this thing the better the product. Goal of trimming is remove not needed code. Goal DynamicallyAccessedMembers(All) is to include all code and not to think about it. So if we go that route it's the sugarcoating fact that we are NOT supporting trimming. There nothing wrong to say that CSLA currently don't support trimming and think about long-term fixes. Once the attributes out in public they part of contract which is would be annoying to fix later on.

I agree with assessment that IL2026 IL2055 IL2057 IL2060 IL2070 IL2072 IL2075 IL21111 IL3050 are hard to swallow as it is. I think we should go creative somehow and think about possible options.

Most of problems which I seen in already annotated code compares from TypeConverter infrastructure. It is convenient thing, but some small things prevent it from being AOT friendly. I would raise a question

• How often custom type converters are writen? How essential that to CSLA?
• Can we have other similar mechanism? Maybe we can go back to dotnet/runtime and present them with case of existing business infrastructure and say, guys, let's discuss what can be done for us and for WinForms guys, I think maybe we can form some sort of synergy.
• Maybe we can improve instantiation logic so it can be done using source gen/interceptors? again, this is somewhat related to TypeConverter infrastructure

@Bowman74 TypeConverter is big offended right now IMO. I manage to push this square peg into round hole and we somehow can fix that.

[RequiresUnreferencedCodeAttribute] I would have to be sleazy as snake to not allow that attribute used indiscriminatedly 😄 I think there some relatively simple way how we can improve things.

One potential plan which we can have is for example start small and make sure that only client CSLA is properly trimmable. and server side would be later on.

ICslaObject seems to be nice idea. I think that should be explored.

One practical solution is to any interested person to try create prototype of it's idea, implement it. Find limitations, comeback to us and share findings. so we can discuss workarounds and specific limitations. I tihnk we are still in learning phase where each interested person don't have even "okay" solution. I almost sure we can do much better

[rockfordlhotka]

I agree with @kant2002 about a complete solution, it is a v10 thing that needs more research and time.

However, as I understand it, if we do nothing right now then it means we don't support MAUI in v9, and I don't want that if there is any way to keep MAUI with a lower impact partial solution.

[kant2002]

@rockfordlhotka can we create sample, run it with and fix all issues on the end-user side? That way we can

• document what user should do in text
• maybe create temporary analyzer with fixes which help user to quickly annotate code.

Obviously that's bad solution since we may teach people bad habits and we should spend time on advocating not to annotate things when not needed. Did somebody try create MAUI as it is?

[Bowman74]

@rockfordlhotka The modest change that you accepted today will fix if for MAUI just plain not running without the invasive changes we are discussing here. So we should be good for .Net MAUI in your next prerelease build based on my testing. I'll test it again when the next nuget is released to make 1000% sure.

What we won't support is native AOT compiled code. Native AOT compiled code will fail if any trim warnings are present (suppressed or not). However, native AOT compilation isn't required to distribute MAUI binaries.

https://learn.microsoft.com/en-us/dotnet/maui/deployment/nativeaot?view=net-maui-9.0

As far as changes to library user code, I feel this is similar to when Microsoft introduced async/await. If CSLA wanted to take advantage of the new pattern, it bubbled throughout CSLA's codebase. If anyone who used your library wanted to take advantage of it, it would bubble into their code as well. It's just the nature of async/await and these trimming annotations that then need to bubble like this. The good news is anyone who doesn't care about trimming can just ignore the warnings just like they could if they called into async/await code without awaiting.

[Bowman74]

@kant2002

Obviously that's bad solution since we may teach people bad habits and we should spend time on advocating not to annotate things when not needed. Did somebody try create MAUI as it is?

I updated the samples. Currently due to some inconsistent trim annotations it won't run on iOS, MacOS or Andriod. However, the changes that need to be made were quite modest and already accepted into main. From what I can see now, we won't need to do the full change just to get it to run but as per my above comment native AOT compilation will not work.

[rockfordlhotka]

@rockfordlhotka The modest change that you accepted today will fix if for MAUI just plain not running without the invasive changes we are discussing here. So we should be good for .Net MAUI in your next prerelease build based on my testing. I'll test it again when the next nuget is released to make 1000% sure.

OK, good. One important question first:

Changing the data portal API either has to happen in 9.0 or 10.0. If I do it now (like today), does that unlock further AOT work for 9.x point releases? Or should the whole thing be deferred to v10?

[Bowman74]

@rockfordlhotka
To be honest I'm not sure sure. After Spanish class I'll implement it and see what we are in for. I just need to know, do you want me to change out the System.Object for all .Net versions with ICslaObject or just .Net versions 8+

[rockfordlhotka]

@rockfordlhotka To be honest I'm not sure sure. After Spanish class I'll implement it and see what we are in for. I just need to know, do you want me to change out the System.Object for all .Net versions with ICslaObject or just .Net versions 8+

I'm torn - bifurcating the user base isn't good. At the same time, if people are using netfx and never plan to become modern in their codebase, I can see an argument to let them continue building up tech debt.

Hmm.

Let's keep things simple - do it for all versions of .NET.

[Bowman74]

Will do Get Outlook for Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Rockford Lhotka ***@***.***> Sent: Thursday, January 2, 2025 10:42:34 AM To: MarimerLLC/csla ***@***.***> Cc: Kevin E. Ford ***@***.***>; Mention ***@***.***> Subject: Re: [MarimerLLC/csla] Compiler Trim Warnings (Issue #4425) @rockfordlhotka<https://github.com/rockfordlhotka> To be honest I'm not sure sure. After Spanish class I'll implement it and see what we are in for. I just need to know, do you want me to change out the System.Object for all .Net versions with ICslaObject or just .Net versions 8+ I'm torn - bifurcating the user base isn't good. At the same time, if people are using netfx and never plan to become modern in their codebase, I can see an argument to let them continue building up tech debt. Hmm. Let's keep things simple - do it for all versions of .NET. — Reply to this email directly, view it on GitHub<#4425 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABBNMJWVUVZB5Y7ILMHFUWD2IV3AVAVCNFSM6AAAAABUPJO5RCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRYGEZTQMBWGM>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[Bowman74]

@rockfordlhotka @kant2002
As I feared, based on how the current DataPortal and serialization work, there are some spots where the RequiresUnreferencedCode attribute is completely unavoidable unless we want to completely rethink things in the framework. We are using .Net methods that are fundamentally incompatible with trimming. For example, we are loading assemblies based on a string, or calling the Type.GetType method and passing a string of what type we want to load.

An example of this is the public static Type GetType(string typeName, bool throwOnError, bool ignoreCase) in MethodCaller.cs where we do many of the forbidden things with no easy way to change how this works.

There is an explanation here on this under Functionality incompatible with trimming where Microsoft states:

This attribute is used when code is fundamentally not trim compatible, or the trim dependency is too complex to explain to the trimmer. This would often be true for methods that dynamically load code for example via xref:System.Reflection.Assembly.LoadFrom(System.String), enumerate or search through all types in an application or assembly for example via xref:System.Type.GetType

There aren't many workarounds for RequiresUnreferencedCode. The best fix is to avoid calling the method at all when trimming and use something else that's trim-compatible.

https://github.com/dotnet/docs/blob/main/docs/core/deploying/trimming/fixing-warnings.md

For now I will use the following annotation, which is spreading like a virus...

#if NET8_0_OR_GREATER
    [RequiresUnreferencedCode("This CSLA functionality is not compatible with trimming.")]
#endif

As an aside, based on how we load data portal methods dynamically in some spots I suspect the ICslaObject will need more than just DynamicallyAccessedMemberTypes.PublicConstructors We need to make sure those methods are not trimmed. I'll noodle more on that.

We also call into System.ComponentModel.TypeDescriptor.GetProperties in several locations in the code and Microsoft themselves decorated that with RequiresUnreferencedCode so all our code that directly or indirectly uses it will need to be so decorated as well.

[rockfordlhotka]

To save time later, please add a string to the resx and use the resource rather than hard coded English.

[Bowman74]

@rockfordlhotka
This is "quickly" getting out of hand. By the time I'm done, I'm not sure much will be trimmable. We use reflection dynamic loading everywhere.

Here is an interesting one. In several places in the code we call MakeGenericMethod. For example, in the ReadOnlyBase's LoadProperty method. To clear those warnings we have to use both RequiresDynamicCode and RequiresUnreferencedCode which basically turn off trimming for anything that calls into this.

Well that cascades into needing to decorate the IManageProperties LoadProperty and LoadPropertyMarkDirty methods. Well pretty much every normal CSLA business type implements that interface. That means every normal CSLA business type will have to be annotated for those methods as well.

Let's just say we do all that annotation and a user is using CLSA and wants to clear trim warnings. Every time a piece of their code calls LoadProperty, it would have to be similarly annotated as well. Then every piece of user code that references those properties would have to also have be annotated with RequiresDynamicCode and RequiresUnreferencedCode.

I think you see where I'm going here.... BTW, BusinessBase's LoadProperty implementation eventually calls into MakeGenericMethod too. The MethodCaller uses MakeGenericMethod as does ManagedObjectBase and GraphMerger. That's just one of the many problematic (from a trimming perspective) methods CSLA uses. I'm not sure this is worth continuing as not only will CLSA's code be completely covered with these annotations but they will simply make the code trimmer friendly by saying almost all CSLA code isn't trimmable and then make the user cover their code with the same to clear the warnings.

[rockfordlhotka]

I expected as much.

I can see where parts of CSLA itself could/should be trimmed if not used - like DataMapper or something.

But it is hard to see where the core rules engine or the user code defining domain types could be trimmed. If that could be trimmed then someone isn't using CSLA or isn't writing domain types properly.

I come back to this core thing right now:

1. I want to ensure CSLA 9 can build MAUI apps
2. I want to defer big changes for AOT to CSLA 10 (if they make sense even)

[Bowman74]

OK, sounds good. Now we know.

When you do a new nuget build with that PR that was accepted yesterday I'll make sure it works with all MAUI platforms. My testing based on my machine says it will, but I want to make sure to test with the actual packages.

BTW, DataMapper calls TypeDescriptor.GetProperties which has it's own annotations that would need to be bubbled up.

[kant2002]

@Bowman74 I notice that you mention that NativeAOT build would not work if any trim warning is present. I can say that not true, it a just that it very likely do not work but because trimmer get rid of some important code. If force trimmer to keep code using DynamicDependency then it will work.

[Bowman74]

@kant2002 I think you may be splitting some hairs. :) I said it will fail. Removing code needed for the application to run is pretty much failing. The native AOT compilers are very aggressive. But yes, we could use DynamicDependency to tell the compiler to keep everything and then tell it to ignore the trim warnings.

As soon as we are running down the path of DynamicDependency our code is just plain not trimmer friendly. Even worse, it may be the user of CSLA that has to do it because DynamicDependency has to be told what types to keep, so it can't be used to stop the user from needing to decorate their CSLA objects. So yes, we could go through our code and tell the trimmer to keep everything CSLA and have the user similarly do it with CSLA derived objects. I think a lot of thought would have to go into exactly what benefits we are providing before we went down that path.