Add support for Trimming/AOT/SingleFile

[lilith]

While certain runtime serializations are pretty hard to do under AOT/Trim, I think most functionality could remain with it enabled, and the rest could work via a source analyzer. Either way, it would be good to annotate the source code into the AOT-compatible and not portions. Drop these into Tomlyn.props (and consider adding a net8.0) target to see the warnings - there aren't many, but they may be essential.

    <EnableTrimAnalyzer>true</EnableTrimAnalyzer>
    <EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

I went about the codebase adding [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.PublicMethods)] to Types passed around, but I think there might need to be a little refactoring to prevent unlimited spread of those annotations.

There's also another option - System.Text.Json already supports source generation and allows turning a JsonNode (tree) into a custom object. I'm unsure how exact the mapping between toml and json is, but converting the parsed toml tree to a jsonnode could be a workaround for some.

[lilith]

I would say that the first step would be adding a .NET 8 target across the projects and to CI, then adding some failing tests that explore exactly what is currently broken under trimming. Would you want to keep .NET 7 or replace it with .NET 6? I see lots of .NET 6 ifdefs. .NET 6 EOL is November 12, 2024, .NET 7 EOL is May 14, 2024.

I don't have deep enough knowledge of the code architecture to refactor what is needed for direct custom model support - the design seems to be partly supportive of the changes but I would likely take it an unhelpful direction without knowing your goals/philosophy. I also don't have that bandwidth, and while my new docker images currently use TOML, I could see switching to JSON as being less costly.

Adding JsonNode translation would introduce a new dependency (although perhaps not on .NET 8?), but could be isolated in code path to allow it to be trimmed out. I'd either do a separate package or make the feature .NET 8 only.

[lilith]

I opened a separate issue for .NET 8: #82

[xoofx]

I went about the codebase adding [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicProperties | DynamicallyAccessedMemberTypes.PublicFields | DynamicallyAccessedMemberTypes.PublicMethods)] to Types passed around, but I think there might need to be a little refactoring to prevent unlimited spread of those annotations.

I dunno how much trouble it will be for Tomlyn, as I forgot the details about the codebase. I tried to do similar stuffs in another OSS project Scriban that is heavily relying on reflection, but it was too laborious to get it working.

Drop these into Tomlyn.props (and consider adding a net8.0) target to see the warnings - there aren't many, but they may be essential.

For .NET 8, I have been using the following (as described here)

<PropertyGroup>
    <IsAotCompatible>true</IsAotCompatible>
</PropertyGroup>

There's also another option - System.Text.Json already supports source generation and allows turning a JsonNode (tree) into a custom object. I'm unsure how exact the mapping between toml and json is, but converting the parsed toml tree to a jsonnode could be a workaround for some.

Yeah, I would probably prefer to avoid this.

[lilith]

Well, I don't think we want to tell downstream libs that it is AOT compatible until we've annotated which APIs are and aren't.

Yeah, I would probably prefer to avoid this.

Avoid source generation or System.Text.Json integration?

[xoofx]

Avoid source generation or System.Text.Json integration?

System.Text.Json

[xoofx]

Well, I don't think we want to tell downstream libs that it is AOT compatible until we've annotated which APIs are and aren't.

Yeah, it was just to suggest that this flag cover the 3 analyzers and we would use it in the end, but probably not during the transition if it takes more than one PR.

[lilith]

Of course. I'd say we could leave the warnings on for now? Tangentially, is there a configuration provider for Tomlyn? https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.configuration.fileconfigurationprovider?view=dotnet-plat-ext-8.0

…

On Fri, Feb 2, 2024, 10:10 AM Alexandre Mutel ***@***.***> wrote: Well, I don't think we want to tell downstream libs that it is AOT compatible until we've annotated which APIs are and aren't. Yeah, it was just to suggest that this flag cover the 3 analyzers and he would use it in the end, but probably not during the transition if it takes more than one PR. — Reply to this email directly, view it on GitHub <#80 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2LHZWH2SRTTJN7ZV65LLYRUMW3AVCNFSM6AAAAABCQKU6JSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRUGMYDANRXGY> . You are receiving this because you authored the thread.Message ID: ***@***.***>