Skip to content

Commit 31ca3b7

Browse files
authored
Fix constructed generic coalescing (#2740)
Preserve symbolic generic arguments through conversion classification and static field member references. Cover the Oahu logging service shape with runtime and ILVerify proofs for user, framework, and nested arguments plus mismatch diagnostics. Fixes #2735
1 parent e0b963e commit 31ca3b7

4 files changed

Lines changed: 279 additions & 1 deletion

File tree

src/Core/CodeAnalysis/Binding/Conversion.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,9 @@ internal static Conversion ClassifyCore(TypeSymbol from, TypeSymbol to, bool all
153153
// conversion as identity rather than reporting a confusing "Cannot
154154
// convert type 'X' to 'X'"-looking error.
155155
if (IsImportedTypeIdentity(from) && IsImportedTypeIdentity(to)
156+
&& (!(from is ImportedTypeSymbol { OpenDefinition: not null, HasSubstitutableTypeArgument: true }
157+
|| to is ImportedTypeSymbol { OpenDefinition: not null, HasSubstitutableTypeArgument: true })
158+
|| TypeSymbol.AreRuntimeEquivalentIgnoringReferenceNullability(from, to))
156159
&& ClrTypeUtilities.AreSame(from.ClrType, to.ClrType))
157160
{
158161
return Conversion.Identity;
@@ -1228,6 +1231,18 @@ internal static Conversion ClassifyCore(TypeSymbol from, TypeSymbol to, bool all
12281231
return Conversion.None;
12291232
}
12301233

1234+
// Issue #2735: an object-erased constructed generic must not fall
1235+
// through to the CLR assignability check below after its symbolic
1236+
// arguments failed the hierarchy/variance checks above. For example,
1237+
// NullLogger<Other>'s probe type NullLogger<object> implements the
1238+
// probe ILogger<object>, but it does not implement ILogger<Store>.
1239+
if (from is ImportedTypeSymbol { OpenDefinition: not null, HasSubstitutableTypeArgument: true }
1240+
&& to is ImportedTypeSymbol mismatchedConstructedTarget
1241+
&& TryGetConstructedGenericShape(mismatchedConstructedTarget, out _, out _))
1242+
{
1243+
return Conversion.None;
1244+
}
1245+
12311246
if (from?.ClrType != null && to?.ClrType != null
12321247
&& !from.ClrType.IsValueType && !to.ClrType.IsValueType
12331248
&& !from.ClrType.IsPointer && !to.ClrType.IsPointer

src/Core/CodeAnalysis/Emit/ImportedMemberRefFactory.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,6 +1682,38 @@ internal MemberReferenceHandle GetFieldReference(FieldInfo field)
16821682
return handle;
16831683
}
16841684

1685+
internal MemberReferenceHandle GetFieldReference(FieldInfo field, TypeSymbol containingTypeSymbol)
1686+
{
1687+
if (!TryNormalizeToSymbolicContainer(containingTypeSymbol, out var openDefinition, out var typeArguments))
1688+
{
1689+
return this.GetFieldReference(field);
1690+
}
1691+
1692+
var openField = openDefinition.GetField(
1693+
field.Name,
1694+
BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic)
1695+
?? throw new InvalidOperationException(
1696+
$"Open generic type '{openDefinition.FullName}' has no field '{field.Name}'.");
1697+
var symbolicView = ImportedTypeSymbol.GetConstructed(
1698+
openDefinition.MakeGenericType(this.GetErasedObjectArgs(openDefinition)),
1699+
openDefinition,
1700+
typeArguments);
1701+
var parentBlob = new BlobBuilder();
1702+
this.signatures.EncodeTypeSymbol(
1703+
new BlobEncoder(parentBlob).TypeSpecificationSignature(),
1704+
symbolicView);
1705+
var parent = this.emitCtx.Metadata.AddTypeSpecification(
1706+
this.emitCtx.Metadata.GetOrAddBlob(parentBlob));
1707+
var sigBlob = new BlobBuilder();
1708+
this.signatures.EncodeClrType(
1709+
new BlobEncoder(sigBlob).FieldSignature(),
1710+
openField.FieldType);
1711+
return this.emitCtx.Metadata.AddMemberReference(
1712+
parent,
1713+
this.emitCtx.Metadata.GetOrAddString(field.Name),
1714+
this.emitCtx.Metadata.GetOrAddBlob(sigBlob));
1715+
}
1716+
16851717
/// <summary>
16861718
/// Issue #649: Gets a MemberRef for a field on a constructed generic type without
16871719
/// calling <c>.GetField()</c> on the closed generic (which throws

src/Core/CodeAnalysis/Emit/MethodBodyEmitter.MemberAccess.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,9 @@ private void EmitClrPropertyAccess(BoundClrPropertyAccessExpression access)
10761076
access.Type);
10771077
break;
10781078
case FieldInfo field:
1079-
var fieldRef = this.outer.memberRefs.GetFieldReference(field);
1079+
var fieldRef = access.StaticContainerType != null
1080+
? this.outer.memberRefs.GetFieldReference(field, access.StaticContainerType)
1081+
: this.outer.memberRefs.GetFieldReference(field);
10801082
this.il.OpCode(isStatic ? ILOpCode.Ldsfld : ILOpCode.Ldfld);
10811083
this.il.Token(fieldRef);
10821084
break;
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
// <copyright file="Issue2735ConstructedGenericCoalesceEmitTests.cs" company="GSharp">
2+
// Copyright (C) GSharp Authors. All rights reserved.
3+
// </copyright>
4+
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Diagnostics;
8+
using System.IO;
9+
using System.Linq;
10+
using Xunit;
11+
12+
namespace GSharp.Compiler.Tests.Emit;
13+
14+
public sealed class Issue2735ConstructedGenericCoalesceEmitTests
15+
{
16+
[Fact]
17+
public void OahuServiceProbe_PreservesUserFrameworkAndNestedCategoryTypesAtRuntime()
18+
{
19+
const string source = """
20+
package Issue2735
21+
import System
22+
import System.Collections.Generic
23+
import Microsoft.Extensions.Logging
24+
import Microsoft.Extensions.Logging.Abstractions
25+
26+
class Store {}
27+
28+
class Service {
29+
private let logger ILogger[Store]
30+
31+
init(logger ILogger[Store]?) {
32+
this.logger = logger ?? NullLogger[Store].Instance
33+
}
34+
35+
func LoggerType() string -> logger!!.GetType().ToString()!!
36+
}
37+
38+
func Main() {
39+
Console.WriteLine(Service(nil).LoggerType())
40+
let framework ILogger[string] = nil ?? NullLogger[string].Instance
41+
Console.WriteLine(framework!!.GetType().ToString())
42+
let nested ILogger[List[Store]] = nil ?? NullLogger[List[Store]].Instance
43+
Console.WriteLine(nested!!.GetType().ToString())
44+
}
45+
""";
46+
47+
using var result = Compile(source, "exe");
48+
Assert.Equal(
49+
"Microsoft.Extensions.Logging.Abstractions.NullLogger`1[Issue2735.Store]\n"
50+
+ "Microsoft.Extensions.Logging.Abstractions.NullLogger`1[System.String]\n"
51+
+ "Microsoft.Extensions.Logging.Abstractions.NullLogger`1[System.Collections.Generic.List`1[Issue2735.Store]]\n",
52+
Run(result.OutputPath));
53+
IlVerifier.Verify(result.OutputPath, additionalReferences: result.References);
54+
}
55+
56+
[Fact]
57+
public void DifferentUserCategory_DoesNotBindThroughObjectErasure()
58+
{
59+
const string source = """
60+
package Issue2735.UserNegative
61+
import Microsoft.Extensions.Logging
62+
import Microsoft.Extensions.Logging.Abstractions
63+
64+
class Store {}
65+
class Other {}
66+
67+
func Bad(logger ILogger[Store]?) ILogger[Store] {
68+
return logger ?? NullLogger[Other].Instance
69+
}
70+
""";
71+
72+
using var result = Compile(source, "library", expectSuccess: false);
73+
Assert.Contains("GS0129", result.Diagnostics, StringComparison.Ordinal);
74+
}
75+
76+
[Fact]
77+
public void DifferentNestedCategory_DoesNotBindThroughObjectErasure()
78+
{
79+
const string source = """
80+
package Issue2735.NestedNegative
81+
import System.Collections.Generic
82+
import Microsoft.Extensions.Logging
83+
import Microsoft.Extensions.Logging.Abstractions
84+
85+
class Store {}
86+
class Other {}
87+
88+
func Bad(logger ILogger[List[Store]]?) ILogger[List[Store]] {
89+
return logger ?? NullLogger[List[Other]].Instance
90+
}
91+
""";
92+
93+
using var result = Compile(source, "library", expectSuccess: false);
94+
Assert.Contains("GS0129", result.Diagnostics, StringComparison.Ordinal);
95+
}
96+
97+
private static CompilationResult Compile(string source, string target, bool expectSuccess = true)
98+
{
99+
var directory = Path.Combine(
100+
AppContext.BaseDirectory,
101+
"issue2735-artifacts",
102+
Guid.NewGuid().ToString("N"));
103+
Directory.CreateDirectory(directory);
104+
var sourcePath = Path.Combine(directory, "test.gs");
105+
var outputPath = Path.Combine(directory, "test.dll");
106+
File.WriteAllText(sourcePath, source);
107+
108+
var references = GetFrameworkReferences();
109+
var args = new List<string>
110+
{
111+
"/out:" + outputPath,
112+
"/target:" + target,
113+
"/targetframework:net10.0",
114+
"/nowarn:GS9100",
115+
};
116+
args.AddRange(references.Select(path => "/reference:" + path));
117+
args.Add(sourcePath);
118+
119+
using var stdout = new StringWriter();
120+
using var stderr = new StringWriter();
121+
var previousOut = Console.Out;
122+
var previousError = Console.Error;
123+
Console.SetOut(stdout);
124+
Console.SetError(stderr);
125+
int exitCode;
126+
try
127+
{
128+
exitCode = Program.Main(args.ToArray());
129+
}
130+
finally
131+
{
132+
Console.SetOut(previousOut);
133+
Console.SetError(previousError);
134+
}
135+
136+
var diagnostics = stdout.ToString() + stderr;
137+
Assert.True(
138+
expectSuccess == (exitCode == 0),
139+
$"expected success: {expectSuccess}; exit code: {exitCode}\n{diagnostics}");
140+
return new CompilationResult(directory, outputPath, references, diagnostics);
141+
}
142+
143+
private static string Run(string outputPath)
144+
{
145+
File.WriteAllText(
146+
Path.ChangeExtension(outputPath, ".runtimeconfig.json"),
147+
"""
148+
{
149+
"runtimeOptions": {
150+
"tfm": "net10.0",
151+
"frameworks": [
152+
{ "name": "Microsoft.NETCore.App", "version": "10.0.0" },
153+
{ "name": "Microsoft.AspNetCore.App", "version": "10.0.0" }
154+
]
155+
}
156+
}
157+
""");
158+
159+
var startInfo = new ProcessStartInfo("dotnet")
160+
{
161+
RedirectStandardOutput = true,
162+
RedirectStandardError = true,
163+
UseShellExecute = false,
164+
WorkingDirectory = Path.GetDirectoryName(outputPath)!,
165+
};
166+
startInfo.ArgumentList.Add("exec");
167+
startInfo.ArgumentList.Add("--runtimeconfig");
168+
startInfo.ArgumentList.Add(Path.ChangeExtension(outputPath, ".runtimeconfig.json"));
169+
startInfo.ArgumentList.Add(outputPath);
170+
171+
using var process = Process.Start(startInfo)
172+
?? throw new InvalidOperationException("Failed to start dotnet exec.");
173+
var stdout = process.StandardOutput.ReadToEnd();
174+
var stderr = process.StandardError.ReadToEnd();
175+
Assert.True(process.WaitForExit(30_000), "dotnet exec timed out.");
176+
Assert.True(
177+
process.ExitCode == 0,
178+
$"exited {process.ExitCode}\nstdout:\n{stdout}\nstderr:\n{stderr}");
179+
return stdout.Replace("\r\n", "\n");
180+
}
181+
182+
private static string[] GetFrameworkReferences()
183+
{
184+
var coreDirectory = Path.GetDirectoryName(typeof(object).Assembly.Location)!;
185+
var sharedDirectory = Directory.GetParent(coreDirectory)!.Parent!.FullName;
186+
var aspNetDirectory = Path.Combine(
187+
sharedDirectory,
188+
"Microsoft.AspNetCore.App",
189+
Path.GetFileName(coreDirectory));
190+
Assert.True(Directory.Exists(aspNetDirectory), $"Missing ASP.NET Core runtime: {aspNetDirectory}");
191+
return Directory.GetFiles(coreDirectory, "*.dll")
192+
.Concat(Directory.GetFiles(aspNetDirectory, "*.dll"))
193+
.ToArray();
194+
}
195+
196+
private sealed class CompilationResult : IDisposable
197+
{
198+
public CompilationResult(
199+
string directory,
200+
string outputPath,
201+
string[] references,
202+
string diagnostics)
203+
{
204+
Directory = directory;
205+
OutputPath = outputPath;
206+
References = references;
207+
Diagnostics = diagnostics;
208+
}
209+
210+
public string Directory { get; }
211+
212+
public string OutputPath { get; }
213+
214+
public string[] References { get; }
215+
216+
public string Diagnostics { get; }
217+
218+
public void Dispose()
219+
{
220+
try
221+
{
222+
System.IO.Directory.Delete(Directory, recursive: true);
223+
}
224+
catch
225+
{
226+
}
227+
}
228+
}
229+
}

0 commit comments

Comments
 (0)