Skip to content

Commit

Permalink
Fix 'hasStaticConstructor' check in MethodCompiler (dotnet#59116)
Browse files Browse the repository at this point in the history
  • Loading branch information
@RikkiGibson
RikkiGibson committed Jan 28, 2022
1 parent 4100c00 commit 674f7d4
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 42 deletions.
76 changes: 34 additions & 42 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,6 @@ private void CompileNamedType(NamedTypeSymbol containingType)
}
}

// Indicates if a static constructor is in the member,
// so we can decide to synthesize a static constructor.
bool hasStaticConstructor = false;

var members = containingType.GetMembers();
for (int memberOrdinal = 0; memberOrdinal < members.Length; memberOrdinal++)
{
Expand Down Expand Up @@ -527,12 +523,6 @@ private void CompileNamedType(NamedTypeSymbol containingType)
default(Binder.ProcessedFieldInitializers);

CompileMethod(method, memberOrdinal, ref processedInitializers, synthesizedSubmissionFields, compilationState);

// Set a flag to indicate that a static constructor is created.
if (method.MethodKind == MethodKind.StaticConstructor)
{
hasStaticConstructor = true;
}
break;
}

Expand Down Expand Up @@ -596,45 +586,47 @@ private void CompileNamedType(NamedTypeSymbol containingType)
}
}

// In the case there are field initializers but we haven't created an implicit static constructor (.cctor) for it,
// (since we may not add .cctor implicitly created for decimals into the symbol table)
// it is necessary for the compiler to generate the static constructor here if we are emitting.
if (_moduleBeingBuiltOpt != null && !hasStaticConstructor && !processedStaticInitializers.BoundInitializers.IsDefaultOrEmpty)
if (containingType.StaticConstructors.IsEmpty)
{
Debug.Assert(processedStaticInitializers.BoundInitializers.All((init) =>
(init.Kind == BoundKind.FieldEqualsValue) && !((BoundFieldEqualsValue)init).Field.IsMetadataConstant));

MethodSymbol method = new SynthesizedStaticConstructor(sourceTypeSymbol);
if (PassesFilter(_filterOpt, method))
// In the case there are field initializers but we haven't created an implicit static constructor (.cctor) for it,
// (since we may not add .cctor implicitly created for decimals into the symbol table)
// it is necessary for the compiler to generate the static constructor here if we are emitting.
if (_moduleBeingBuiltOpt != null && !processedStaticInitializers.BoundInitializers.IsDefaultOrEmpty)
{
CompileMethod(method, -1, ref processedStaticInitializers, synthesizedSubmissionFields, compilationState);
Debug.Assert(processedStaticInitializers.BoundInitializers.All((init) =>
(init.Kind == BoundKind.FieldEqualsValue) && !((BoundFieldEqualsValue)init).Field.IsMetadataConstant));

// If this method has been successfully built, we emit it.
if (_moduleBeingBuiltOpt.GetMethodBody(method) != null)
MethodSymbol method = new SynthesizedStaticConstructor(sourceTypeSymbol);
if (PassesFilter(_filterOpt, method))
{
_moduleBeingBuiltOpt.AddSynthesizedDefinition(sourceTypeSymbol, method.GetCciAdapter());
CompileMethod(method, -1, ref processedStaticInitializers, synthesizedSubmissionFields, compilationState);

// If this method has been successfully built, we emit it.
if (_moduleBeingBuiltOpt.GetMethodBody(method) != null)
{
_moduleBeingBuiltOpt.AddSynthesizedDefinition(sourceTypeSymbol, method.GetCciAdapter());
}
}
}
}

// If there is no explicit or implicit .cctor and no static initializers, then report
// warnings for any static non-nullable fields. (If there is no .cctor, there
// shouldn't be any initializers but for robustness, we check both.)
if (!hasStaticConstructor &&
processedStaticInitializers.BoundInitializers.IsDefaultOrEmpty &&
_compilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion() &&
containingType is { IsImplicitlyDeclared: false, TypeKind: TypeKind.Class or TypeKind.Struct or TypeKind.Interface } &&
ReportNullableDiagnostics)
{
NullableWalker.AnalyzeIfNeeded(
this._compilation,
new SynthesizedStaticConstructor(containingType),
GetSynthesizedEmptyBody(containingType),
_diagnostics.DiagnosticBag,
useConstructorExitWarnings: true,
initialNullableState: null,
getFinalNullableState: false,
finalNullableState: out _);
// If there is no explicit or implicit .cctor and no static initializers, then report
// warnings for any static non-nullable fields. (If there is no .cctor, there
// shouldn't be any initializers but for robustness, we check both.)
if (processedStaticInitializers.BoundInitializers.IsDefaultOrEmpty &&
_compilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion() &&
containingType is { IsImplicitlyDeclared: false, TypeKind: TypeKind.Class or TypeKind.Struct or TypeKind.Interface } &&
ReportNullableDiagnostics)
{
NullableWalker.AnalyzeIfNeeded(
this._compilation,
new SynthesizedStaticConstructor(containingType),
GetSynthesizedEmptyBody(containingType),
_diagnostics.DiagnosticBag,
useConstructorExitWarnings: true,
initialNullableState: null,
getFinalNullableState: false,
finalNullableState: out _);
}
}

// compile submission constructor last so that synthesized submission fields are collected from all script methods:
Expand Down
119 changes: 119 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/UninitializedNonNullableFieldTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2310,6 +2310,13 @@ static C()
}
";
var comp = CreateCompilation(new[] { source1, source2 }, options: WithNullableEnable());

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[0], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify();

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[1], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify();

comp.VerifyDiagnostics(
// (4,28): warning CS0414: The field 'C.s1' is assigned but its value is never used
// static readonly string s1;
Expand All @@ -2333,12 +2340,124 @@ partial class C
}
";
var comp = CreateCompilation(new[] { source1, source2 }, options: WithNullableEnable());

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[0], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify(
// (4,37): warning CS8602: Dereference of a possibly null reference.
// static readonly string Field1 = Field2.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Field2").WithLocation(4, 37));

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[1], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify();

comp.VerifyDiagnostics(
// (4,37): warning CS8602: Dereference of a possibly null reference.
// static readonly string Field1 = Field2.ToString(); // 1
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Field2").WithLocation(4, 37));
}

[Fact]
public void StaticInitializers_MultipleFiles_03()
{
var source1 = @"
public partial class Class1
{
public static readonly string Value1;
static Class1 ()
{
Value1 = string.Empty;
Value2 = string.Empty;
}
}
";
var source2 = @"
public partial class Class1
{
public static readonly string Value2;
}
";
var comp = CreateCompilation(new[] { source1, source2 }, options: WithNullableEnable());

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[0], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify();

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[1], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify();

comp.VerifyDiagnostics();
}

[Fact]
public void StaticInitializers_MultipleFiles_04()
{
var source1 = @"
public partial class Class1
{
public static readonly string Value1; // 1
}
";
var source2 = @"
public partial class Class1
{
public static readonly string Value2; // 2
}
";
var comp = CreateCompilation(new[] { source1, source2 }, options: WithNullableEnable());

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[0], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify(
// (4,35): warning CS8618: Non-nullable field 'Value1' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
// public static readonly string Value1; // 1
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Value1").WithArguments("field", "Value1").WithLocation(4, 35));

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[1], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify(
// (4,35): warning CS8618: Non-nullable field 'Value2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
// public static readonly string Value2; // 2
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Value2").WithArguments("field", "Value2").WithLocation(4, 35));

comp.VerifyDiagnostics(
// (4,35): warning CS8618: Non-nullable field 'Value1' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
// public static readonly string Value1; // 1
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Value1").WithArguments("field", "Value1").WithLocation(4, 35),
// (4,35): warning CS8618: Non-nullable field 'Value2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
// public static readonly string Value2; // 2
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Value2").WithArguments("field", "Value2").WithLocation(4, 35));
}

[Fact]
public void StaticInitializers_MultipleFiles_05()
{
var source1 = @"
public partial class Class1
{
public static readonly string Value1 = ""a"";
}
";
var source2 = @"
public partial class Class1
{
public static readonly string Value2; // 1
}
";
var comp = CreateCompilation(new[] { source1, source2 }, options: WithNullableEnable());

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[0], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify();

comp.GetDiagnosticsForSyntaxTree(CompilationStage.Compile, comp.SyntaxTrees[1], filterSpanWithinTree: null, includeEarlierStages: true)
.Verify(
// (4,35): warning CS8618: Non-nullable field 'Value2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
// public static readonly string Value2; // 1
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Value2").WithArguments("field", "Value2").WithLocation(4, 35));

comp.VerifyDiagnostics(
// (4,35): warning CS8618: Non-nullable field 'Value2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
// public static readonly string Value2; // 1
Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "Value2").WithArguments("field", "Value2").WithLocation(4, 35));
}

[Fact]
[WorkItem(1090263, "https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/1090263")]
public void PropertyNoGetter()
Expand Down

0 comments on commit 674f7d4

Please sign in to comment.