Skip to content

Param nullchecking binding #36247

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Jun 11, 2019
Merged

Param nullchecking binding #36247

merged 20 commits into from
Jun 11, 2019

Conversation

@fayrose
Copy link
Contributor

@fayrose fayrose commented Jun 7, 2019

Working cases for binding parameter nullchecking.

@fayrose fayrose requested a review from a team as a code owner June 7, 2019 20:01
@fayrose fayrose assigned fayrose and unassigned fayrose Jun 7, 2019
@fayrose
Copy link
Contributor Author

fayrose commented Jun 7, 2019

Currently adding asserts for IsNullChecked.

agocke
agocke reviewed Jun 10, 2019
View reviewed changes
src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs
}
}
internal override bool IsNullChecked
=> this.CSharpSyntaxNode.ExclamationToken.Kind() == SyntaxKind.ExclamationToken;
Copy link
Member

@agocke agocke Jun 10, 2019

Choose a reason for hiding this comment

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

Can ExclamationToken be null? Also, I think the standard way of checking if a token is present is to call IsMissing instead of checking the Kind.

Copy link
Contributor Author

@fayrose fayrose Jun 10, 2019

Choose a reason for hiding this comment

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

I believe it's SyntaxKind.None if there's no token.

Copy link
Member

@RikkiGibson RikkiGibson Jun 11, 2019

Choose a reason for hiding this comment

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

I'm also wondering about when IsMissing is preferred over Kind() == SyntaxKind.None. It seems like IsMissing is mainly for when the token's absence is a syntax error, not when the token is optional.

agocke
agocke reviewed Jun 10, 2019
View reviewed changes
@agocke
Copy link
Member

agocke commented Jun 10, 2019

Looks like there some copy/paste errors around the asserts, should be simple to fix up

agocke
agocke approved these changes Jun 11, 2019
View reviewed changes
Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

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

LGTM

@fayrose fayrose merged commit 3e72bb0 into dotnet:features/param-nullchecking Jun 11, 2019
@fayrose fayrose deleted the param-nullchecking-binding branch June 11, 2019 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RikkiGibson RikkiGibson RikkiGibson approved these changes

@agocke agocke agocke approved these changes

Assignees

No one assigned

Labels

Area-Compilers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fayrose @agocke @RikkiGibson