-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Use built-in inline array types when possible #80557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Instead of synthesizing inline array types, we want to use the ones from the framework when possible (in .NET 10). Contributes to dotnet#74538.
<NetVSShared>net8.0</NetVSShared> | ||
<NetRoslynBuildHostNetCoreVersion>net6.0</NetRoslynBuildHostNetCoreVersion> | ||
<NetRoslynNext>net9.0</NetRoslynNext> | ||
<NetRoslynNext>net10.0</NetRoslynNext> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only projects using this value are now the Emit3
unit tests, and BuildBoss
.
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs
Outdated
Show resolved
Hide resolved
@dotnet/roslyn-compiler for a second review |
Done with review pass (commit 4) #Closed |
@AlekseyTs for another review pass |
} | ||
|
||
// InlineArray types must be sequential, as we do arithmetic on them. | ||
var startingOffset = WellKnownType.System_Runtime_CompilerServices_InlineArray2 - 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entire method is Conditional(Debug)
.
return typeSymbol; | ||
} | ||
|
||
internal static bool TryGetWellKnownType(CSharpCompilation compilation, WellKnownType type, BindingDiagnosticBag diagnostics, Location location, [NotNullWhen(true)] out NamedTypeSymbol? typeSymbol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return false; | ||
} | ||
|
||
ReportUseSite(typeSymbol, diagnostics, location); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{ | ||
typeSymbol = compilation.GetWellKnownType(type); | ||
Debug.Assert((object)typeSymbol != null, "Expect an error type if well-known type isn't found"); | ||
if (typeSymbol.GetUseSiteInfo().DiagnosticInfo?.Severity == DiagnosticSeverity.Error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | ||
}; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to add a test verifying that dependencies are properly tracked. For example, something like the following:
- Create library with a built-in inline array type
- Create a compilation referencing that library and using a scenario that uses the type implicitly
3, Verify that the set of used assembly references includes reference to the library with the inline array type.
Done with review pass (commit 5) |
Instead of synthesizing inline array types, we want to use the ones from the framework when possible (in .NET 10). Contributes to #74538.