Skip to content

Conversation

@AlekseyTs
Copy link
Contributor

Fixes #14761.

@dotnet/roslyn-compiler Please review.

GoTo EndBlock

Case Else
Throw ExceptionUtilities.UnexpectedValue(token.Kind)
Copy link
Member

Choose a reason for hiding this comment

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

Am I right in assuming this fail safe would've caught this case and prevented a successful compilation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, before the change we were allowing all kinds of things to pass through the Case Else.

' if there is a line-If without Else up the block context chain, or
' if we are inside a single-line statement lambda and the 'Else' terminates the lambda.
Debug.Assert(BlockKind = SyntaxKind.SingleLineElseClause)
Debug.Assert(PrevBlock.BlockKind = SyntaxKind.SingleLineIfStatement)
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 wrong to have a property which can only be contextually invoked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The property can always be invoked, this asserts an invariant in the way block contexts are nested for line-if. They are always nested this way.

@AlekseyTs
Copy link
Contributor Author

@dotnet/roslyn-compiler Please review.

@AdamSpeight2008
Copy link
Contributor

@AlekseyTs Could you provide some short code examples of what was allowed and now isn't?

@AlekseyTs
Copy link
Contributor Author

Could you provide some short code examples of what was allowed and now isn't?

The issue referenced at the top, #14761, has short example. I also added a bunch of unit-tests, you can compare compiler's behavior for them for before and after the change.

@cston
Copy link
Contributor

cston commented Nov 1, 2016

LGTM

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