Skip to content

Simplify return type parsing#18206

Merged
2 commits merged into
masterfrom
parseReturnType
Sep 1, 2017
Merged

Simplify return type parsing#18206
2 commits merged into
masterfrom
parseReturnType

Conversation

@ghost

@ghost ghost commented Sep 1, 2017

Copy link
Copy Markdown

There was some dead code around due to checking returnToken after it had already been checked.

@ghost ghost requested a review from sandersn September 1, 2017 16:28
@ghost ghost changed the title Parse return type Simplify return type parsing Sep 1, 2017

@sandersn sandersn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like the top-level structure change (function + if/elif/elif/else + return) but the contents of the if-branches are weird -- one is empty, and one returns where the others don't. The current version is weird too, particularly the saving of start/length and the naming of backwardToken, and arguably the abstraction over returnToken. Overall, though, I don't think this change is worth making.

@ghost ghost merged commit 7a4c3da into master Sep 1, 2017
@ghost ghost deleted the parseReturnType branch September 1, 2017 17:58
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants