Skip to content
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

Fix allow ref like types for synthesized TypeParameterSymbols in Ad-Hoc Delegates #74714

Closed
wants to merge 10 commits into from

Conversation

bernd5
Copy link
Contributor

@bernd5 bernd5 commented Aug 12, 2024

Currently synthesized TypeParameterSymbols for array params return AllowsRefLikeType=true, which is not true.
An array can not contain ref-structs.

fixes: #74823

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Aug 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Aug 12, 2024
@bernd5 bernd5 marked this pull request as ready for review August 20, 2024 14:13
@bernd5 bernd5 requested a review from a team as a code owner August 20, 2024 14:13
@jaredpar
Copy link
Member

Do we have a bug for this?

@bernd5
Copy link
Contributor Author

bernd5 commented Aug 20, 2024

See #74823

@@ -32,6 +32,40 @@ internal static void VerifyDiagnostics(string source, params DiagnosticDescripti
[CompilerTrait(CompilerFeature.LocalFunctions)]
public class LocalFunctionTests : LocalFunctionsTestBase
{
[Fact]
public void TestAdHocDelegateWithParams_NotAllowRefLikeTest()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like this should be in a different file than LocalFunctionTests, for example RefStructInterfacesTests seems better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know where to define it - that would be a better place or something like "allowRefStructTests.cs"


namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class AllowsRefLikeTypeSymbolVisitor : SymbolVisitor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other cases than just params T[] that need to be handled specially?

  • If yes, we should have tests for these cases.
  • If no, we don't need a visitor, we could simply check just that one case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not as far as I know

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to simplify the check

@bernd5 bernd5 closed this Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Blocked Community The pull request was submitted by a contributor who is not a Microsoft employee. untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anonymous-Params-delegate is generated incorrectly with AllowRefStructFlag
4 participants