Skip to content

Warn on cctors annotated with RUC if a field triggers its marking#1889

Merged
mateoatr merged 5 commits intodotnet:mainfrom
mateoatr:rucCctorField
Mar 15, 2021
Merged

Warn on cctors annotated with RUC if a field triggers its marking#1889
mateoatr merged 5 commits intodotnet:mainfrom
mateoatr:rucCctorField

Conversation

@mateoatr
Copy link
Contributor

Whenever a static member is referenced, static constructors on the declared type are called. If the cctor is annotated with RequiresUnreferencedCode, this should warn.

{
}

public static int field = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be better not to use static fields to check that types without beforefieldinit modifiers work as expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think moving the field initialization into the static .cctor should be enough, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test coverage should be better. Does the code print the warning in this case?

new C ().Foo ();

class C
{
	[RequiresUnreferencedCode ("Message")]
    static C ()
    {
    }
    
    public void Foo ()
    {
    }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, AFAIK Roslyn won't make the type beforefieldinit in any of the above cases - the explicit static ctor in C# doesn't have the relaxed semantics allowed by beforefieldinit. To test with beforefieldinit you need to avoid writing a static ctor and let Roslyn generate the .cctor from the static field initializers:

class C {
    static int field = RUC();
    [RequiresUnreferencedCode]
    static int RUC() => 0;
}

Mateo Torres Ruiz added 2 commits March 12, 2021 09:20
public static int AnnotatedMethod () => 42;
}

[LogContains ("IL2026: Mono.Linker.Tests.Cases.RequiresCapability.RequiresUnreferencedCodeCapability.TypeIsBeforeFieldInit..cctor():" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we use LogContains here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @sbomer puts above, Roslyn generates a .cctor and annotates the type with beforefieldinit, this static constructor initializes the field and thus calls AnnotatedMethod, triggering the warning from within the .cctor. Because of this, there's no place where we could put the ExpectedWarning attribute (the call is made from generated code).

@mateoatr mateoatr merged commit 252c665 into dotnet:main Mar 15, 2021
agocke pushed a commit to dotnet/runtime that referenced this pull request Nov 16, 2022
…tnet/linker#1889)

* Cctor marked because of accessed field should trigger warning

* Ignore testcase in analyzer

* Add tests
Move field initialization to cctor

* Add beforefieldinit test to list of tests ignored by the analyzer

* Fix testname

Commit migrated from dotnet/linker@252c665
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants