Structs cannot be annotated as `RequiresUnreferencedCode`

[m-redding]

Description

Structs cannot be annotated as RequiresUnreferencedCode. I'm trying to make a library AOT compatible, and we have multiple structs that are not compatible with trimming. In most cases, we can just annotate all incompatible methods with RequiresUnreferencedCode. However, we have 2 cases where we have an overload for ToString which is incompatible with trimming. Since we cannot annotate the method or struct the warning will never be resolved.

Reproduction Steps

internal struct Sample
{
        public override string ToString()
        {
                // Reflection-based serialization code not compatible with trimming
        }
}

Expected behavior

Ability to annotate structs as RequiresUnreferencedCode

Actual behavior

Adding RequiresUnreferencedCode attribute to a struct results in CS0592

Regression?

No response

Known Workarounds

For the ToString() overload case, none. For the general case, every method must be annotated.

Configuration

No response

Other information

No response

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Structs cannot be annotated as RequiresUnreferencedCode. I'm trying to make a library AOT compatible, and we have multiple structs that are not compatible with trimming. In most cases, we can just annotate all incompatible methods with RequiresUnreferencedCode. However, we have 2 cases where we have an overload for ToString which is incompatible with trimming. Since we cannot annotate the method or struct the warning will never be resolved.

Reproduction Steps

internal struct Sample
{
        public override string ToString()
        {
                // Reflection-based serialization code not compatible with trimming
        }
}

Expected behavior

Ability to annotate structs as RequiresUnreferencedCode

Actual behavior

Adding RequiresUnreferencedCode attribute to a struct results in CS0592

Regression?

No response

Known Workarounds

For the ToString() overload case, none. For the general case, every method must be annotated.

Configuration

No response

Other information

No response

Author:	m-redding
Assignees:	-
Labels:	untriaged, area-Tools-ILLink, needs-area-label
Milestone:	-

[jkotas]

@vitek-karas @sbomer Is this just an oversight?

[sbomer]

The annotation was intentionally only supported on classes according to the reasoning here: #50122.

If we allow this on structs, we would have to warn on instance methods too, plus it would introduce problems when unsafe instance methods get called indirectly through interfaces (the tool would not be able to see that at all).

[eerhardt]

How can an "unsafe" ToString on a struct be annotated correctly? You can't put RequiresUnreferencedCode on ToString(). You can't put RequiresUnreferencedCode on the struct itself. What are your other options beyond suppressing the warning?

[sbomer]

As far as I know, there's currently no "safe" way to annotate that, without changes to the analysis that we do. In the API review for RUC, we discussed maybe adding support for annotating structs, but it would be a new feature. To catch all cases, we'd need to decide where to warn in this example:

using System.Diagnostics.CodeAnalysis;

class Program {
    static void Main() {
        S s;
        CallInterfaceMethod(s);
    }

    static void CallInterfaceMethod(I i) => i.M();
}

interface I {
    void M();
}

struct S : I {
    public S() => throw new Exception("ctor called");

    public void M() => Helper.RUC();
}

class Helper {
    [RequiresUnreferencedCode("Helper")]
    public static void RUC() => throw new Exception("RUC called");
}

Note that the struct ctor is never called, so the warning would probably need to be about the declaration in Main.

[MichalStrehovsky]

On a high level, we'd need to detect when the struct is boxed, or a target of a constrained call. The analysis in IL Linker assumes that structs are boxed implicitly (the moment a struct type is reachable, we mark all the virtuals, etc.). This is a shortcut - if we could detect boxing or constrained calls, we could treat these the same as reference types. The rest are "details", but of course the devil is in them.

Alternatively, we could say that a struct annotated as RUC warns every time someone touches it in any way (use as a generic argument, make an array of it, etc.). It will produce a lot of warnings - the warnings we try to avoid by the careful treatment of RUC on classes - even in code that is not too problematic in general.

[eerhardt]

Connecting the dots here, the problematic code in Azure.Core is:

https://github.com/Azure/azure-sdk-for-net/blob/c4fb4c52117622c2107163ba6d3efbb20743836d/sdk/core/Azure.Core/src/DynamicData/MutableJsonChange.cs#L52-L61

This code is eventually called from MutableJsonChange's ToString().

@m-redding - In this case, since this struct is internal, I would annotate the constructor as RequiresUnreferencedCode and then suppress the warning in the ToString. The object? Value readonly property (which is the root of being incompatible with trimming here) can only be set by the constructor. So only callers using the constructor will be able to use this struct.

Following all the callsites up, the only place I see this exposed publicly is through APIs like this:

        public static dynamic ToDynamicFromJson(this BinaryData utf8Json)
        {
            DynamicDataOptions options = new DynamicDataOptions();
            return utf8Json.ToDynamicFromJson(options);
        }

Which will never be trim or AOT compatible. So we should just mark all this code as RequiresUnreferencedCode, and any Azure library that wants to be trim/AOT compatible will need to use alternate APIs.