Skip to content

Commit

Permalink
Forbid types with required members from substituting for where T : new()
Browse files Browse the repository at this point in the history
  • Loading branch information
333fred committed Mar 30, 2022
1 parent 49a53a5 commit 3f9e71d
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 7 deletions.
7 changes: 4 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1378,16 +1378,17 @@ private static bool HasPublicParameterlessConstructor(NamedTypeSymbol type, bool
{
Debug.Assert(type.TypeKind is TypeKind.Class or TypeKind.Struct);

// PROTOTYPE(req): Adjust for required members
bool hasAnyRequiredMembers = type.HasAnyRequiredMembers;

foreach (var constructor in type.InstanceConstructors)
{
if (constructor.ParameterCount == 0)
{
return constructor.DeclaredAccessibility == Accessibility.Public;
return constructor.DeclaredAccessibility == Accessibility.Public
&& (!hasAnyRequiredMembers || constructor.HasSetsRequiredMembers);
}
}
return synthesizedIfMissing;
return synthesizedIfMissing && !hasAnyRequiredMembers;
}

/// <summary>
Expand Down
9 changes: 8 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -523,13 +523,20 @@ internal bool HasRequiredMembersError
}
}

/// <summary>
/// Returns true if there are any required members. Prefer calling this over checking <see cref="AllRequiredMembers"/> for empty, as
/// this will avoid calculating base type requirements if not necessary.
/// </summary>
internal bool HasAnyRequiredMembers => HasDeclaredRequiredMembers || !AllRequiredMembers.IsEmpty;

/// <summary>
/// The full list of all required members for this type, including from base classes. If <see cref="HasRequiredMembersError"/> is true,
/// this returns empty.
/// </summary>
/// <remarks>
/// Do not call this API if all you need are the required members declared on this type. Use <see cref="GetMembers()"/> instead, filtering for
/// required members, instead of calling this API.
/// required members, instead of calling this API. If you only need to determine whether this type or any base types have required members, call
/// <see cref="HasAnyRequiredMembers"/>, which will avoid calling this API if not required.
/// </remarks>
internal ImmutableSegmentedDictionary<string, Symbol> AllRequiredMembers
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,6 @@ internal static bool HasCopyConstructorSignature(MethodSymbol member)

protected sealed override bool HasSetsRequiredMembersImpl
// If the record type has a required members error, then it does have required members of some kind, we emit the SetsRequiredMembers attribute.
=> !ContainingType.AllRequiredMembers.IsEmpty || ContainingType.HasRequiredMembersError;
=> ContainingType.HasAnyRequiredMembers || ContainingType.HasRequiredMembersError;
}
}
120 changes: 120 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3634,4 +3634,124 @@ void validate(ModuleSymbol module)
}
}
}

[Theory]
[InlineData("struct")]
[InlineData("class")]
public void ForbidRequiredAsNew_NoInheritance(string typeKind)
{
var code = $$"""
M<C>();
void M<T>() where T : new()
{
}
{{typeKind}} C
{
public required int Prop1 { get; set; }
}
""";

var comp = CreateCompilationWithRequiredMembers(code);
comp.VerifyDiagnostics(
// (1,1): error CS0310: 'C' must be a non-abstract type with a public parameterless constructor in order to use it as parameter 'T' in the generic type or method 'M<T>()'
// M<C>();
Diagnostic(ErrorCode.ERR_NewConstraintNotSatisfied, "M<C>").WithArguments("M<T>()", "T", "C").WithLocation(1, 1)
);
}

[Theory]
[CombinatorialData]
public void ForbidRequiredAsNew_Inheritance(bool useMetadataReference)
{
var @base = """
public class Base
{
public required int Prop1 { get; set; }
}
""";

var code = """
M<Derived>();
void M<T>() where T : new()
{
}
class Derived : Base
{
}
""";

var comp = CreateCompilationWithRequiredMembers(new[] { @base, code });
comp.VerifyDiagnostics(
// (1,1): error CS0310: 'Derived' must be a non-abstract type with a public parameterless constructor in order to use it as parameter 'T' in the generic type or method 'M<T>()'
// M<Derived>();
Diagnostic(ErrorCode.ERR_NewConstraintNotSatisfied, "M<Derived>").WithArguments("M<T>()", "T", "Derived").WithLocation(1, 1)
);

var baseComp = CreateCompilationWithRequiredMembers(@base);
CompileAndVerify(baseComp).VerifyDiagnostics();

comp = CreateCompilation(code, references: new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() });
comp.VerifyDiagnostics(
// (1,1): error CS0310: 'Derived' must be a non-abstract type with a public parameterless constructor in order to use it as parameter 'T' in the generic type or method 'M<T>()'
// M<Derived>();
Diagnostic(ErrorCode.ERR_NewConstraintNotSatisfied, "M<Derived>").WithArguments("M<T>()", "T", "Derived").WithLocation(1, 1)
);
}

[Theory]
[InlineData("class")]
[InlineData("struct")]
public void AllowRequiredAsNew_SetsRequiredMembersOnConstructor(string typeKind)
{
var code = $$"""
using System.Diagnostics.CodeAnalysis;
M<C>();
void M<T>() where T : new()
{
}
{{typeKind}} C
{
public required int Prop1 { get; set; }
[SetsRequiredMembers]
public C()
{
}
}
""";

var comp = CreateCompilationWithRequiredMembers(code);
CompileAndVerify(comp).VerifyDiagnostics();
}

[Fact]
public void AllowRequiredAsNew_IndirectionViaStruct()
{
var code = """
M1<C>();
void M1<T>() where T : struct
{
M2<T>();
}
void M2<T>() where T : new()
{
}
struct C
{
public required int Prop1 { get; set; }
}
""";

var comp = CreateCompilationWithRequiredMembers(code);
CompileAndVerify(comp).VerifyDiagnostics();
}
}
4 changes: 2 additions & 2 deletions src/Compilers/Test/Utilities/CSharp/CSharpTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ public UnmanagedCallersOnlyAttribute() { }
namespace System.Runtime.CompilerServices
{
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Field | AttributeTargets.Property, Inherited = false, AllowMultiple = false)]
public class RequiredMemberAttribute : Attribute
public sealed class RequiredMemberAttribute : Attribute
{
public RequiredMemberAttribute()
{
Expand All @@ -642,7 +642,7 @@ public RequiredMemberAttribute()
namespace System.Diagnostics.CodeAnalysis
{
[AttributeUsage(AttributeTargets.Constructor, Inherited = false, AllowMultiple = false)]
public class SetsRequiredMembersAttribute : Attribute
public sealed class SetsRequiredMembersAttribute : Attribute
{
public SetsRequiredMembersAttribute()
{
Expand Down

0 comments on commit 3f9e71d

Please sign in to comment.