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

return, break and continue expressions #20269

Closed
wants to merge 1 commit into from

Conversation

alrz
Copy link
Contributor

@alrz alrz commented Jun 16, 2017

Prototype for dotnet/csharplang#176

@sharwell sharwell added PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jun 16, 2017
@alrz
Copy link
Contributor Author

alrz commented Jun 16, 2017

@sharwell This is not "for personal review only". I'd actually like this to be reviewed if it gets past the ldm.

@sharwell sharwell removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jun 16, 2017
@alrz alrz force-pushed the features/jmp-expr branch 4 times, most recently from 4cf8969 to 4f0ef86 Compare June 21, 2017 18:01
@jcouv
Copy link
Member

jcouv commented Jun 21, 2017

@alrz I will bring this up in LDM next week.
If we can squeeze it in the agenda, we can get feedback on the language proposal itself. Otherwise, I'll get an ETA for discussion.

I'd suggest waiting for feedback from LDM before investing more time in code (the prototype you have so far will help though). If supportive, I expect LDM will focus on language questions first (do we want this in the language? how does it fit and how is it spec'ed?). After that I will expect there will be scheduling questions (as we'll need to cover semantic model APIs, IDE scenarios such as completion and colorizing, and more).

@jcouv
Copy link
Member

jcouv commented Jul 7, 2017

LDM discussed this briefly, but did not come to a conclusion. The next session is in 3 or 4 weeks. Hold on.

@sharwell sharwell added Community The pull request was submitted by a contributor who is not a Microsoft employee. and removed Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Aug 31, 2017
@alrz
Copy link
Contributor Author

alrz commented Sep 1, 2017

Since there is no language proposal for this feature yet I'll close this untill LDM confirm.

@alrz alrz closed this Sep 1, 2017
@alrz alrz reopened this Sep 8, 2017
bool hasErrors = false;
if (!IsJumpExpressionInProperContext(node))
{
diagnostics.Add(ErrorCode.ERR_ThrowMisplaced, node.ReturnKeyword.GetLocation());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could I change ERR_ThrowMispalced message to A {0} expression is not allowed in this context. to be used for all jump expressions?

@@ -13,6 +13,7 @@ internal enum ConversionKind : byte
ImplicitNumeric,
ImplicitEnumeration,
ImplicitThrow,
ImplicitJump,
Copy link
Contributor Author

@alrz alrz Sep 8, 2017

Choose a reason for hiding this comment

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

Could we merge ImplicitThrow and ImplicitJump together? (one issue is IsThrow public property)

<Rule Id="RS0022" Action="Error" />
<Rule Id="RS0024" Action="Error" />
<Rule Id="RS0025" Action="Error" />
<Rule Id="RS0026" Action="Error" />
<Rule Id="RS0027" Action="Error" />
<Rule Id="RS0027" Action="None" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is disabled to make return-expression's expression optional; , otherwise it complains about the default parameter. (bug?)

@jcouv
Copy link
Member

jcouv commented Oct 9, 2017

@alrz Thanks for your patience. To give you a heads-up, the csharplang championed issue was pushed until after 8.0 in terms of priority. I'll let you check out today's LDM notes for more details, once they are published.

@sharwell
Copy link
Member

sharwell commented Nov 8, 2017

@alrz I used the blocked label instead as a way to indicate this is not ready to merge. I'm trying to make sure as many Community PRs get merged as possible and the more focused I can make my search results, the less likely I'll be to miss one that could have been merged. 😄

@gafter
Copy link
Member

gafter commented Apr 20, 2018

Since the LDM has not approved any such language feature, we cannot accept this PR.

@gafter gafter closed this Apr 20, 2018
@gafter
Copy link
Member

gafter commented Apr 20, 2018

Correction: this has a champion.

@gafter gafter reopened this Apr 20, 2018
@jcouv
Copy link
Member

jcouv commented May 22, 2018

LDM discussed this feature yesterday and decided we do not want to do the feature.
There are possible interactions with expression blocks and other advanced control flow features, both of which aren't designed yet.
See upcoming LDM meeting notes for more details.

@jcouv jcouv closed this May 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Area-Language Design Blocked cla-already-signed Community The pull request was submitted by a contributor who is not a Microsoft employee. Language-C#
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants