Annotate is native aot compatible

[OleRoss]

Hi, I did not find any information about AOT compatibility on this project, but I got some IL warnings when building a project of mine.

I annotated both UnitsNet and UnitsNet.NumberExtensions projects. I skipped UnitsNet.Serialization.JsonNet as this would probably need bigger changes.

Things done:

• Added an Enum.GetValues method, which calls a typed overload if targeting a newer dotnet version.
• Added generic overloads to methods that needed it
• Added attributes which warn the user about incompatibilities to Native AOT compilation

I tried to make minimal changes only, but possibly breaking changes are

• the addition of a struct constraint to generic enum-constrained methods
• a different way to generate RegisterDefaultConversions

Feel free to merge or reject, but I would be happy to have those annotations included in the project to make life easier when working with NativeAOT.

[angularsen]

Hi I think this looks promising. I don't have any experience with AOT myself, but at a glance it looks good. Any thoughts @lipchev ?

@OleRoss Could you please target release/v6 branch?
We need to port it there anyway, and we're kind of freezing v5 to focus on v6. There was also a minor breaking change with the new struct constraint added.

[lipchev]

I did a quick test on my unreleased branch- and as far as I can tell there was only a single .cs file that needed to change:

        private static TAttribute? GetAttribute<
#if NET
            [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor | DynamicallyAccessedMemberTypes.PublicFields)]
#endif
            TAttribute>(ITypeDescriptorContext? context) where TAttribute : UnitAttributeBase
        {

I would probably annotate directly the class itself, as from the above code I can barely tell where's the start and end of the parameter definition. 😄

I also did a quick refactoring on my SystemTextJson converters, and they (the generic versions) appear to compile without warnings.

I haven't got the C++ stuff installed, so wasn't able to actually test if it really works, but my UnitsNetAPI.AOT sample appears to compile without warnings.

[OleRoss]

Hi @angularsen, @lipchev, thanks for taking a look at the pr.

I rebased the PR on the release/v6 branch. @lipchev did you do some changes I just purged?

Additionally, could you elaborate I would probably annotate directly the class itself, as from the above code I can barely tell where's the start and end of the parameter definition. 😄, please? I am not sure how I could annotate the class?

[lipchev]

Hi @angularsen, @lipchev, thanks for taking a look at the pr.

I rebased the PR on the release/v6 branch. @lipchev did you do some changes I just purged?

I merged #1507 an hour or so ago, but I think your merge conflicts are probably coming from the file changes in the v5 branch. Changing the branch of a PR is a real hassle- if I were you, I'd would probably start from a clean pull from release/v6.

Additionally, could you elaborate I would probably annotate directly the class itself, as from the above code I can barely tell where's the start and end of the parameter definition. 😄, please? I am not sure how I could annotate the class?

I'm no expert, but this appears to work (I copy pasted it from the System.Text.Json source).

#if NET
[RequiresDynamicCode("The native code for this instantiation might not be available at runtime.")]
[RequiresUnreferencedCode("If some of the generic arguments are annotated (either with DynamicallyAccessedMembersAttribute, or generic constraints), trimming can't validate that the requirements of those annotations are met.")]
#endif
public class JsonQuantityConverter : JsonConverterFactory

[OleRoss]

@lipchev I updated the branch to include your merged PR.
Unfortunatelly, I cannot use Enum.GetValues(unitEnumType) with AOT, but because of the todo comment I see two options:

• Return empty or throw an exception (would make the Enum.GetValues call redundant
• Split the logic into two methods, one with a generic overload and one with an attribute

For now, I throw an exception (one idea from the comment) but that leads to failing unit tests. I would wait until the todo is resolved before doing more.

[lipchev]

Yeah, that's one of the few TODO's I've got left.

@angularsen In #1507 You kind a left it at "Reasonable, but out of scope of this PR" - how do you think we should handle this, throw or empty?

[angularsen]

I think we should definitely throw, with some helpful exception message instructing to add the QuantityInfo to the UnitAbbreviationsCache. I guess this normally only happens for custom quantities, or when creating a blank new abbreviation cache for whatever reason.

It seems we don't yet support adding QuantityInfo to an existing cache, so we probably also need a way add custom quantities to the default singleton cache:

UnitAbbreviationsCache.Default.AddQuantity(HowMuch.Info); // QuantityInfo
UnitAbbreviationsCache.Default.MapUnitToAbbreviation(HowMuchUnit.AShitTon, culture, "my shiny new abbreviation");

[angularsen]

Just to add, although we probably could continue mapping units to abbreviations without a QuantityInfo, it feels more coherent to reuse the QuantityInfo and UnitInfo structures for this. A unit always belongs to a quantity, and I think it's fair to require both.

Also, this decision will probably affect approximately zero people out there.

[angularsen]

@OleRoss master merged in release/v6 today so you'll have to recreate this PR anyway now, sorry!
It had to be done anyway, with the huge diff between the two branches it's easier to just manually redo your changes in a brand new PR on the-now-updated master branch.