Champion "return, break and continue expressions"

[MadsTorgersen]

• Proposal added (see Proposal: return, break and continue expressions #176)
• Discussed in LDM (Added: C# Language Design Notes for Jun 27, 2017 #722)
• Decision in LDM
• Finalized (done, rejected, inactive)
• Spec'ed

Update: Let's consider again when we have match expressions, where they may become more useful.

[HaloFour]

And goto 😀

We can pretend that it would only ever be used to break out of nested loops or to jump to a specific case label within a switch statement.

[jcouv]

@alrz has closed his Roslyn PR pending LDM input. Ali, you may want to re-open it now that Mads is driving it. Are there any language questions you'd like to raise?

[alrz]

@jcouv Sure. I'd like to bring up the issue with throw expressions on the right-hand-side of logical operators which caused it to be disallowed. Maybe it makes sense for jump expressions to be allowed in that context. Otherwise this would be very similar to throw expressions except for cases where existing rules wouldn't work out, for example, if nesting is permitted, we shouldn't report unreachable code after return e ?? break;.

[gafter]

@alrz A return expression would have the same precedence issues as a throw expression, which is what caused issues with the logical operators. What is your poster-child use case for either?

[alrz]

I thought about using it with || in loops but turns out local functions can be a better choice there. other than that I don't think there would be any use case that this could lead to a better alternative.

[jcouv]

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.

Tagging @MadsTorgersen if anything to add, and to close the championed issue.

[HaloFour]

@jcouv

That's unfortunate. I hope that the notes contain some details around this theoretical future feature that isn't designed/championed yet.

[MadsTorgersen]

Raw notes here: https:/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-05-21.md

One of the potential features this rubs against is "block expressions", the idea that you can more generally nest statements inside of an expression. If we ever do those, then they would solve the scenario in a more general way (albeit probably with slightly more syntax).

Most of the future risk we see, though is around the meaning of return. If we ever want to do a version of non-local returns (i.e. returns out of an enclosing body, rather than the immediately enclosing one), some proposals are able to use return expressions for that. We don't want to force our future hand on this, so to speak.

[AndreasHassing]

That's too bad - I think this proposal would be very beneficial in my ASP MVC Core controller actions:

var thing = await thingService.GetThingAsync(id);
if (thing == null) { return RedirectToIndex(); }

// becomes
var thing = await thingService.GetThingAsync(id) ?? return RedirectToIndex();

However, I completely respect the thought process: if we introduce this now, it may cripple us later - let's wait.

But if it turns out that block expressions can be implemented alongside this, please reconsider it 😁.

[Jason5Lee]

This is very useful for nullable value type. Without it, I have to write

double? valueNullable = expr;
if (valueNullable == null) return expr;
double value = (double)valueNullable;

With it, I could only need to write

double value = expr ?? return expr;

[Richiban]

@Jason5Lee Your example doesn't make any sense;

double? valueNullable = expr;
if (valueNullable != null) return expr;
double value = (double)valueNullable;

On line 3 valueNullable will always have the value null, since if it's not null you returned on the previous line.

What are you trying to do?

Don't forget we now have this syntax too:

if (!(expr is {} d))
{
    return;
}

// d is now in scope and is of type `double`

[jnm2]

Also in C# 9:

if (expr is not { } d) return;

[Jason5Lee]

@Richiban Oops I write the condition wrong (that's why I need strict null check).
Thanks for your example I never knew I could do that.

[Pzixel]

Update: Let's consider again when we have match expressions, where they may become more useful.

So the time has come? Thin may be revisited?

[ArcanoxDragon]

This feature would be excellent in a context where multiple bits of optional data are required for a given key, such as something like this (the Kotlin language actually allows almost this exact syntax):

Foo? GetFoo(int key);
Bar? GetBar(int key);

foreach (int key in SomeCollection) {
    Foo foo = GetFoo(key) ?? continue;
    Bar bar = GetBar(key) ?? continue;

    // now do stuff knowing that foo and bar are not null
}

Accomplishing the above using pattern matching works, but is not immediately obvious to the eye that a variable of type Foo is introduced that's guaranteed non-null:

foreach (int key in SomeCollection) {
    if (GetFoo(key) is not { } foo) continue;
    if (GetBar(key) is not { } bar) continue;

    // now do stuff knowing that foo and bar are not null
}

In the latter version, one has to visually parse the if statements to see that a variable is introduced in a pattern. Granted, is not Foo foo can be used, but default ReSharper inspections recommend against this (for a good reason; if the return type of GetFoo or GetBar was changed, the pattern will always evaluate to false, whereas a variable assignment would be a compiler warning/error).

[MithrilMan]

Is there any possibility that this feature will be taken into consideration?

[CyrusNajmabadi]

@MithrilMan yes. This proposal is championed and is being considered for a future release.

[Abrynos]

I can see some more great uses of this, if we put it in the context of yield:

IEnumerable<Foo> GetSeveralFoos() {
    // some other code

    Foo? foo = GetFoo();
    yield return foo ?? continue; // continues iteration if foo is null

    Foo? bar = GetOtherFoo();
    yield return bar ?? break; // stops the iteration if bar is null

    // even more code
}

[acaly]

I can see some more great uses of this, if we put it in the context of yield:

IEnumerable<Foo> GetSeveralFoos() {
    // some other code

    Foo? foo = GetFoo();
    yield return foo ?? continue; // continues iteration if foo is null

    Foo? bar = GetOtherFoo();
    yield return bar ?? break; // stops the iteration if bar is null

    // even more code
}

I am not sure what you mean by "the iteration" here. continue or break should not be used inside enumerable methods to control the enumeration of the returned IEnumerable. They should only control the local loop block they are in. For example:

IEnumerable<Foo> GetSeveralFoos() {
    foreach (var foo in GetAllFoos()) //The foreach loop.
    {
        if (...) break; //Break the foreach loop above.
        //or with this proposal implemented:
        yield return foo ?? break; //Break the foreach loop above.
    }
}

[ADIX7]

It would be also helpful in case a new object must be created with a constructor that has multiple parameters.

So instead of this

if ("HeaderName1" is var s1 && GetHeader(s1) is not { } headerValue1) return new MissingHeaderError(s1);
if ("HeaderName2" is var s2 && GetHeader(s2) is not { } headerValue2) return new MissingHeaderError(s2);
if ("HeaderName3" is var s3 && GetHeader(s3) is not { } headerValue3) return new MissingHeaderError(s3);
if ("HeaderName4" is var s4 && GetHeader(s4) is not { } headerValue4) return new MissingHeaderError(s4);
if ("HeaderName5" is var s5 && GetHeader(s5) is not { } headerValue5) return new MissingHeaderError(s5);
if ("HeaderName6" is var s6 && GetHeader(s6) is not { } headerValue6) return new MissingHeaderError(s6);
if ("HeaderName7" is var s7 && GetHeader(s7) is not { } headerValue7) return new MissingHeaderError(s7);
if ("HeaderName8" is var s8 && GetHeader(s8) is not { } headerValue8) return new MissingHeaderError(s8);
if ("HeaderName9" is var s9 && GetHeader(s9) is not { } headerValue9) return new MissingHeaderError(s9);

return new SomeSpecialRecord(
	headerValue1,
	headerValue2,
	headerValue3,
	headerValue4,
	headerValue5,
	headerValue6,
	headerValue7,
	headerValue8,
	headerValue9
);

I could use this

return new SomeSpecialRecord(
	GetHeader("HeaderName1") ?? return new MissingHeaderError("HeaderName1"),
	GetHeader("HeaderName2") ?? return new MissingHeaderError("HeaderName2"),
	GetHeader("HeaderName3") ?? return new MissingHeaderError("HeaderName3"),
	GetHeader("HeaderName4") ?? return new MissingHeaderError("HeaderName4"),
	GetHeader("HeaderName5") ?? return new MissingHeaderError("HeaderName5"),
	GetHeader("HeaderName6") ?? return new MissingHeaderError("HeaderName6"),
	GetHeader("HeaderName7") ?? return new MissingHeaderError("HeaderName7"),
	GetHeader("HeaderName8") ?? return new MissingHeaderError("HeaderName8"),
	GetHeader("HeaderName9") ?? return new MissingHeaderError("HeaderName9"),
);

Half the lines and also easier to read.

[theunrepentantgeek]

I'm generally in favour of having return expressions (neutral on the other kinds), but code like this would never get past a code review.

As written, the code starts with

return new SomeSpecialRecord(

This naturally leads any reader of the code to conclude that an instance of SomeSpecialRecord must be returned if execution reaches this point.

But that's not true. The bait-and-switch of having return ?? new MissingHeaderError() concealed within the expression would be a trip-hazard (or worse) for any maintaining software engineer.

In fact, I'm wondering whether there should be a restriction that a return expression cannot be used within a return statement.

[theunrepentantgeek]

Aside: I find it weird that the first example jumps through complicated hoops to avoid repetition of the string constants for HeaderName1 and friends, while the second version repeats them with nary a concern.

In fact, with a smarter helper method, the code can be simplified considerably using syntax that's available today:

var error = 
    FindHeader("HeaderName1", out var headerValue1)
    ?? FindHeader("HeaderName2", out var headerValue2)
    ?? FindHeader("HeaderName3", out var headerValue3)
    ?? FindHeader("HeaderName4", out var headerValue4)
    ?? FindHeader("HeaderName5", out var headerValue5)
    ?? FindHeader("HeaderName6", out var headerValue6)
    ?? FindHeader("HeaderName7", out var headerValue7)
    ?? FindHeader("HeaderName8", out var headerValue8)
    ?? FindHeader("HeaderName9", out var headerValue9);
  
if error != null {
    return error;
}

return new SomeSpecialRecord(
	headerValue1,
	headerValue2,
	headerValue3,
	headerValue4,
	headerValue5,
	headerValue6,
	headerValue7,
	headerValue8,
	headerValue9
);

[Abrynos]

In fact, with a smarter helper method, the code can be simplified considerably using syntax that's available today

In fact we can write this whole thing as a single statement:

return FindHeader("HeaderName1", out var headerValue1)
    ?? FindHeader("HeaderName2", out var headerValue2)
    ?? FindHeader("HeaderName3", out var headerValue3)
    ?? FindHeader("HeaderName4", out var headerValue4)
    ?? FindHeader("HeaderName5", out var headerValue5)
    ?? FindHeader("HeaderName6", out var headerValue6)
    ?? FindHeader("HeaderName7", out var headerValue7)
    ?? FindHeader("HeaderName8", out var headerValue8)
    ?? FindHeader("HeaderName9", out var headerValue9)
    ?? new SomeSpecialRecord(
	headerValue1,
	headerValue2,
	headerValue3,
	headerValue4,
	headerValue5,
	headerValue6,
	headerValue7,
	headerValue8,
	headerValue9
    );