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

Implement more validation for inline array element access and conversion scenarios #68146

Merged
merged 3 commits into from
May 12, 2023

Conversation

AlekseyTs
Copy link
Contributor

@AlekseyTs AlekseyTs commented May 9, 2023

Relates to test plan #67826

@AlekseyTs AlekseyTs requested review from jjonescz and cston May 9, 2023 21:44
@AlekseyTs AlekseyTs requested a review from a team as a code owner May 9, 2023 21:44
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label May 9, 2023
@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler For the second review

1 similar comment
@AlekseyTs
Copy link
Contributor Author

@cston, @dotnet/roslyn-compiler For the second review

{
var buffer = m.GlobalNamespace.GetTypeMember("Buffer");

Assert.True(buffer.HasInlineArrayAttribute(out int length));
Copy link
Member

@cston cston May 11, 2023

Choose a reason for hiding this comment

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

Assert.True(buffer.HasInlineArrayAttribute(out int length));

Why is the type with two fields recognized as a valid inline array type? #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is the type with two fields recognized as a valid inline array type?

It is not. This API is not determining whether the type is a valid inline array type. It only indicates whether it has a valid attribute. The final check is performed by TryGetInlineArrayElementField, which returns the only field

}
}
}
}

return elementType;
if (elementField is not null && elementField.ContainingType.IsGenericType)
Copy link
Member

@cston cston May 11, 2023

Choose a reason for hiding this comment

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

elementField.ContainingType.IsGenericType

Are we testing nested types, with and without generic type parameters? #Resolved

Copy link
Contributor Author

@AlekseyTs AlekseyTs May 11, 2023

Choose a reason for hiding this comment

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

Are we testing nested types, with and without generic type parameters?

We are not at the moment, but IsGenericType check covers the scenario. I will add a test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests.

{
private T _element0;
}
}
Copy link
Member

@cston cston May 11, 2023

Choose a reason for hiding this comment

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

Consider using this type:

var c = new C();
c.F[2] = 1;
Console.WriteLine(c.F[2]);

class C
{
    Enclosing<int>.Buffer<string> F;
}
``` #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider using this type: ...

Done

@AlekseyTs AlekseyTs enabled auto-merge (squash) May 12, 2023 01:45
@AlekseyTs AlekseyTs merged commit b833e97 into dotnet:features/InlineArrays May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers New Feature - Inline Arrays 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.

3 participants