Skip to content

Commit

Permalink
Fixed a stack overflow issue when visiting symbols for type parameter…
Browse files Browse the repository at this point in the history
… variance.
  • Loading branch information
MeltyPlayer committed Apr 23, 2024
1 parent 772189e commit 55b032d
Show file tree
Hide file tree
Showing 7 changed files with 293 additions and 99 deletions.
11 changes: 11 additions & 0 deletions Schema Build Tests/readOnly/VarianceTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,15 @@ public partial interface IOkayToAddVariance<T> {
[Const]
public void PassGetSet(ISet<T> list);
}


[GenerateReadOnly]
public partial interface IFinCollection<out T>;

[GenerateReadOnly]
public partial interface ISubTypeDictionary<TKey, TValue>
: IFinCollection<(TKey Key, TValue Value)> {
[Const]
TValueSub Get<TValueSub>(TKey key) where TValueSub : TValue;
}
}
2 changes: 2 additions & 0 deletions Schema Tests/binary/generator/VarianceGeneratorTests.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using NUnit.Framework;

using schema.readOnly;


namespace schema.binary.text {
internal class VarianceGeneratorTests {
Expand Down
2 changes: 1 addition & 1 deletion Schema Tests/readOnly/BasicReadOnlyGeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public partial class SimpleGenerics<T1, T2> : IReadOnlySimpleGenerics<T1, T2> {
T1 IReadOnlySimpleGenerics<T1, T2>.Foo<T3, T4>(T1 t1, T2 t2, T3 t3, T4 t4) => Foo<T3, T4>(t1, t2, t3, t4);
}
public interface IReadOnlySimpleGenerics<T1, T2> {
public interface IReadOnlySimpleGenerics<T1, in T2> {
public T1 Foo<T3, T4>(T1 t1, T2 t2, T3 t3, T4 t4);
}
}
Expand Down
4 changes: 2 additions & 2 deletions Schema Tests/readOnly/ConstraintTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public partial interface ICircular<TMutable, TReadOnly, TImpl> : IReadOnlyCircul
TMutable IReadOnlyCircular<TMutable, TReadOnly, TImpl>.Foo(in TImpl other) => Foo(in other);
}
public interface IReadOnlyCircular<out TMutable, TReadOnly, TImpl> where TMutable : ICircular<TMutable, TReadOnly, TImpl>, TReadOnly where TReadOnly : IReadOnlyCircular<TMutable, TReadOnly, TImpl> {
public interface IReadOnlyCircular<TMutable, TReadOnly, TImpl> where TMutable : ICircular<TMutable, TReadOnly, TImpl>, TReadOnly where TReadOnly : IReadOnlyCircular<TMutable, TReadOnly, TImpl> {
public TMutable Foo(TReadOnly other);
public TMutable Foo(in TImpl other);
}
Expand Down Expand Up @@ -98,7 +98,7 @@ public partial class SubConstraint<T1, T2> : IReadOnlySubConstraint<T1, T2> {
T2 IReadOnlySubConstraint<T1, T2>.Bar => Bar;
}
public interface IReadOnlySubConstraint<out T1, out T2> where T2 : T1 {
public interface IReadOnlySubConstraint<T1, out T2> where T2 : T1 {
public T1 Foo<S>(S s) where S : T1;
public T2 Bar { get; }
}
Expand Down
74 changes: 65 additions & 9 deletions Schema Tests/readOnly/VarianceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,17 @@ public interface IReadOnlyWrapper<T> {
public void TestDoesNotAddContravarianceForSet() {
ReadOnlyGeneratorTestUtil.AssertGenerated(
"""
using schema.readOnly;
using System.Collections.Generic;
using schema.readOnly;
using System.Collections.Generic;
namespace foo.bar {
[GenerateReadOnly]
public partial interface IWrapper<T> {
[Const]
public void Method(ISet<T> foo);
}
namespace foo.bar {
[GenerateReadOnly]
public partial interface IWrapper<T> {
[Const]
public void Method(ISet<T> foo);
}
""",
}
""",
"""
namespace foo.bar {
public partial interface IWrapper<T> : IReadOnlyWrapper<T> {
Expand All @@ -240,5 +240,61 @@ public interface IReadOnlyWrapper<T> {
""");
}

[Test]
public void TestDoesNotAddVarianceWhenUsedAsTypeConstraint() {
ReadOnlyGeneratorTestUtil.AssertGenerated(
"""
using schema.readOnly;
namespace foo.bar {
public partial interface IFinCollection<in T>;
[GenerateReadOnly]
public partial interface ISubTypeDictionary<T1, T2>
: IFinCollection<T2>
where T2 : T1;
}
""",
"""
namespace foo.bar {
public partial interface ISubTypeDictionary<T1, T2> : IReadOnlySubTypeDictionary<T1, T2>;
public interface IReadOnlySubTypeDictionary<T1, in T2> : IFinCollection<T2> where T2 : T1;
}
""");
}

[Test]
public void TestDoesNotAddVarianceWhenUsedAsMethodConstraint() {
ReadOnlyGeneratorTestUtil.AssertGenerated(
"""
using schema.readOnly;
namespace foo.bar {
public partial interface IFinCollection<out T>;
[GenerateReadOnly]
public partial interface ISubTypeDictionary<TKey, TValue>
: IFinCollection<(TKey Key, TValue Value)> {
[Const]
TValueSub Get<TValueSub>(TKey key) where TValueSub : TValue;
}
}
""",
"""
namespace foo.bar {
public partial interface ISubTypeDictionary<TKey, TValue> : IReadOnlySubTypeDictionary<TKey, TValue> {
TValueSub IReadOnlySubTypeDictionary<TKey, TValue>.Get<TValueSub>(TKey key) => Get<TValueSub>(key);
}
public interface IReadOnlySubTypeDictionary<TKey, TValue> : IFinCollection<(TKey Key, TValue Value)> {
public TValueSub Get<TValueSub>(TKey key) where TValueSub : TValue;
}
}
""");
}
}
}
93 changes: 6 additions & 87 deletions Schema/src/readOnly/ReadOnlyTypeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,11 @@ public static string GetGenericParametersWithVarianceForReadOnlyVersion(
var allParentTypes
= symbol.GetBaseTypes().Concat(symbol.AllInterfaces).ToArray();

var set = new TypeParameterSymbolVarianceSet(
typeParameters,
allParentTypes,
constMembers);

var sb = new StringBuilder();
sb.Append("<");
for (var i = 0; i < typeParameters.Length; ++i) {
Expand All @@ -626,9 +631,7 @@ var allParentTypes

var variance = typeParameter.Variance;
if (variance == VarianceKind.None) {
variance = typeParameter.GetExpandedVarianceForReadonlyVersion(
allParentTypes,
constMembers);
variance = set.AllowedVariance(typeParameter);
}

sb.Append(variance switch {
Expand All @@ -643,90 +646,6 @@ var allParentTypes
return sb.ToString();
}

public static VarianceKind GetExpandedVarianceForReadonlyVersion(
this ITypeParameterSymbol typeParameterSymbol,
IReadOnlyList<INamedTypeSymbol> allParentTypes,
IReadOnlyList<IMethodSymbol> constMembers) {
var matchingTypeArguments
= allParentTypes
.SelectMany(NamedTypeSymbolUtil.GetTypeParamsAndArgs)
.Where(paramAndArg => paramAndArg.typeArgumentSymbol.Name ==
typeParameterSymbol.Name)
.ToArray();

if (matchingTypeArguments.Length == 0 ||
matchingTypeArguments.All(
paramAndArg => paramAndArg.typeParameterSymbol.Variance ==
VarianceKind.Out)) {
return constMembers.Any(
constMember
=> constMember.Parameters.Any(
p => p.Type.DependsOn(typeParameterSymbol)) ||
constMember.ReturnType.DependsOnButHasWrongVariance(
typeParameterSymbol,
VarianceKind.Out))
? VarianceKind.None
: VarianceKind.Out;
}

if (matchingTypeArguments.Length == 0 ||
matchingTypeArguments.All(
paramAndArg => paramAndArg.typeParameterSymbol.Variance ==
VarianceKind.In)) {
return constMembers.Any(
constMember
=> constMember.ReturnType.DependsOn(typeParameterSymbol) ||
constMember.Parameters.Any(
p => p.Type.DependsOnButHasWrongVariance(
typeParameterSymbol,
VarianceKind.In)))
? VarianceKind.None
: VarianceKind.In;
}

return VarianceKind.None;
}

public static bool DependsOn(this ITypeSymbol typeSymbol,
ITypeParameterSymbol typeParameterSymbol)
=> typeSymbol.DependsOnImpl_(null, typeParameterSymbol, out _);

private static bool DependsOnImpl_(
this ITypeSymbol typeSymbol,
ITypeParameterSymbol? thisTypeParameterSymbol,
ITypeParameterSymbol otherTypeParameterSymbol,
out ITypeParameterSymbol? match) {
if (typeSymbol.IsSameAs(otherTypeParameterSymbol)) {
match = thisTypeParameterSymbol;
return true;
}

if (typeSymbol.IsGenericZipped(out var typeParamsAndArgs)) {
foreach (var (typeParam, typeArg) in typeParamsAndArgs) {
if (typeArg.DependsOnImpl_(typeParam,
otherTypeParameterSymbol,
out match)) {
return true;
}
}
}

match = null;
return false;
}

public static bool DependsOnButHasWrongVariance(
this ITypeSymbol typeSymbol,
ITypeParameterSymbol otherTypeSymbol,
VarianceKind expectedVarianceKind) {
if (typeSymbol.DependsOnImpl_(null, otherTypeSymbol, out var match) &&
match != null) {
return match.Variance != expectedVarianceKind;
}

return false;
}

public static string GetCStyleCastToReadOnlyIfNeeded(
this ITypeSymbol sourceSymbol,
ITypeSymbol symbol,
Expand Down
Loading

0 comments on commit 55b032d

Please sign in to comment.