KEMBAR78
Implement more validation for inline array element access and conversion scenarios by AlekseyTs · Pull Request #68146 · dotnet/roslyn · GitHub
Skip to content

Conversation

@AlekseyTs
Copy link
Contributor

@AlekseyTs AlekseyTs commented May 9, 2023

Relates to test plan #67826

@AlekseyTs AlekseyTs requested review from cston and jjonescz May 9, 2023 21:44
@AlekseyTs AlekseyTs requested a review from a team as a code owner May 9, 2023 21:44
@ghost ghost 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
Contributor

@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
Contributor

@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
Contributor

@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 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