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

Support scoped keyword in for statement. #64154

Merged
merged 2 commits into from
Sep 23, 2022
Merged

Conversation

AlekseyTs
Copy link
Contributor

else
{
this.EatToken();
isDeclaration = ScanType() != ScanTypeFlags.NotType && this.CurrentToken.Kind == SyntaxKind.IdentifierToken;
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Sep 20, 2022

Choose a reason for hiding this comment

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

code below uses 'IsTrueIdentifier', any concern about this direct .IdentifierToken check? #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.

code below uses 'IsTrueIdentifier', any concern about this direct .IdentifierToken check?

I don't think so. I think that, at most, that can affect how we parse error scenarios and I am fine to prioritize scoped type for now.

decl = decl.Update(
CheckFeatureAvailability(decl.Type, MessageID.IDS_FeatureRefFor),
decl.Variables);
decl = decl.Update(declType, decl.Variables);
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Sep 20, 2022

Choose a reason for hiding this comment

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

nit, could just do the update no matter what. it will already test that the type actually changed before updating. #WontFix

@AlekseyTs
Copy link
Contributor Author

@cston, @RikkiGibson, @dotnet/roslyn-compiler Please review

@RikkiGibson RikkiGibson self-assigned this Sep 22, 2022
@AlekseyTs
Copy link
Contributor Author

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

{
var source =
@"for (scoped s1 = default;;) {
for (ref scoped s2 = ref s1;;) {break;} // 1
Copy link
Contributor

@RikkiGibson RikkiGibson Sep 22, 2022

Choose a reason for hiding this comment

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

consider indenting to make the nesting clear. Also, it looks like there is no diagnostic given on this line. #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.

it looks like there is no diagnostic given on this line.

The same as in the test above. Comments aren't there to convey any expectations. They are there to make it easier to map baseline to the source. Expectations are set by baselines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will address indenting in the next PR

@AlekseyTs AlekseyTs merged commit bea5f26 into dotnet:main Sep 23, 2022
@ghost ghost added this to the Next milestone Sep 23, 2022
@Cosifne Cosifne modified the milestones: Next, 17.4 P3 Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants