Skip to content

Test netmodules containing module initializers#44228

Merged
RikkiGibson merged 4 commits intodotnet:features/module-initializersfrom
RikkiGibson:mi-netmodules
May 16, 2020
Merged

Test netmodules containing module initializers#44228
RikkiGibson merged 4 commits intodotnet:features/module-initializersfrom
RikkiGibson:mi-netmodules

Conversation

@RikkiGibson
Copy link
Member

Related to #40500.

Test based on CompilationEmitTests.MultipleNetmodulesWithAnonymousTypes.

Will think on whether there are more netmodule scenarios we need to test.

@RikkiGibson RikkiGibson requested a review from a team as a code owner May 13, 2020 20:59
@RikkiGibson RikkiGibson added the Test Test failures in roslyn-CI label May 13, 2020
Copy link
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

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

:shipit:

namespace System.Runtime.CompilerServices { public class ModuleInitializerAttribute : System.Attribute { } }";
var comp1 = CreateCompilation(s1, options: TestOptions.ReleaseModule.WithModuleName("A"), parseOptions: s_parseOptions);
comp1.VerifyDiagnostics();
var ref1 = comp1.EmitToImageReference();
Copy link
Contributor

@AlekseyTs AlekseyTs May 14, 2020

Choose a reason for hiding this comment

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

var ref1 = comp1.EmitToImageReference(); [](start = 12, length = 40)

I think for each compilation we want to verify whether we emit the <Module>..cctor #Closed

var comp6 = CreateCompilation(s6, options: TestOptions.ReleaseExe.WithModuleName("C"), parseOptions: s_parseOptions, references: new[] { ref1, ref2 });
comp6.VerifyDiagnostics();
CompileAndVerify(comp6, expectedOutput: "13");
}
Copy link
Contributor

@AlekseyTs AlekseyTs May 14, 2020

Choose a reason for hiding this comment

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

} [](start = 8, length = 1)

I think it would be good to cover scenarios when the main module of the assembly also has an initializer. #Closed

@AlekseyTs
Copy link
Contributor

AlekseyTs commented May 14, 2020

Done with review pass (iteration 1) #Closed

[ModuleInitializer]
public static void Init()
{
Console.Write(1);
Copy link
Contributor

@AlekseyTs AlekseyTs May 14, 2020

Choose a reason for hiding this comment

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

1 [](start = 22, length = 1)

Does this number conflict with the one printed by ref1? Please use unique numbers to avoid any confusion. #Closed

void validateNoModuleInitializer(ModuleSymbol module)
{
var moduleType = module.ContainingAssembly.GetTypeByMetadataName("<Module>");
Assert.Null(moduleType.GetMember<MethodSymbol>(".cctor"));
Copy link
Contributor

@AlekseyTs AlekseyTs May 14, 2020

Choose a reason for hiding this comment

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

Assert.Null(moduleType.GetMember(".cctor")); [](start = 16, length = 58)

Please add Assert.Equal(MetadataImportOptions.All, ((PEModuleSymbol)module).ImportOptions); as we do in other similar validators. #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 2), with a couple of suggestions for the test.

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (iteration 3)

@RikkiGibson RikkiGibson merged commit d5faa63 into dotnet:features/module-initializers May 16, 2020
@RikkiGibson RikkiGibson deleted the mi-netmodules branch May 16, 2020 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Test Test failures in roslyn-CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants