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

Control-flow in expressions #2025

Open
ds84182 opened this issue Dec 13, 2021 · 35 comments
Open

Control-flow in expressions #2025

ds84182 opened this issue Dec 13, 2021 · 35 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@ds84182
Copy link

ds84182 commented Dec 13, 2021

Not to be confused with "control flow as expressions", which is a different feature.

I've been writing a lot of NNBD Dart recently, and I've been running into this messy construct a lot when it comes to field promotion:

final myField = this.myField;
if (myField == null) return;
// Continue to use myField...

I would love to write:

final myField = this.myField ?? return;

This construct is very common in languages that treat statements as expressions, like Kotlin and Rust.

Proposal

Like throw, other control flow statements like return, break, and continue should be allowed in expression position as an expression that results in the type Never.

There's still an open question whether the precedence should be corrected to flow more naturally, but this might open up some syntactic ambiguities. The examples presented here assume there is no precedence issue.

Examples

Summing up all non-null integers in a list:

int? tryReduce(List<int?> list) {
  int sum = 0;
  for (var element in list) {
    sum += element ?? continue;
  }
  return sum;
}

Parsing data without using exceptions as control flow:

abstract class ParserContext {
  T? parse<T>(String context, T Function(ParserContext) parser);
  Token? char(int charCode);
}

IntTuple? parseIntTuple(ParserContext context) {
  context.char($lparen) ?? return null;
  var left = context.parse('left integer', parseInt) ?? return null;
  context.char($comma) ?? return null;
  var right = context.parse('right integer', parseInt) ?? return null;
  context.char($rparen) ?? return null;

  return IntTuple(left, right);
}

Concerns

  • This may make control flow less apparent

Most to all syntax highlighters highlight control flow keywords with a contrasting bold style.

  • There is no prior art in Dart for control flow keywords within expressions

Both throw and await are allowed in expressions. throw will always jump execution to the nearest exception handler, which may be way outside of the current function. await may never return, if the waited Future never completes or throws an error.

Other Languages

Rust

Rust allows the use of the "try-operator" to implement this in a way that is less syntatically-intrusive. While it does allow for statements in expression position, the lack of a ternary or elvis operator means the above forms cannot be used. Still, we can rewrite the examples in Rust:

fn try_reduce(slice: &[Option<u32>]) -> u32 {
  // NOTE: Since Option<T> implements IntoIterator we can simply use flatten
  slice.iter().flatten().reduce(|a, b| a + b)
}

trait ParserContext {
  fn parse<T, F>(&mut self, context: Cow<'static, str>, parser: F) -> Option<T>
    where F: for<'a> FnOnce(&'a Self) -> Option<T>;

  fn char(&mut self, char_code: u8) -> Option<Token>;
}

fn parse_int_tuple(context: &mut impl ParserContext) -> Option<(u32, u32)> {
  // NOTE: Try operator (in this context) is equivalent to `match expr { Some(x) => x, None => return None }`
  context.char(b'(')?;
  let left = context.parse("left integer".into(), parse_int)?;
  context.char(b',')?;
  let right = context.parse("right integer".into(), parse_int)?;
  context.char(b')')?;
  (left, right)
}
@ds84182 ds84182 added the feature Proposed language feature that solves one or more problems label Dec 13, 2021
@munificent
Copy link
Member

OK, so I think the request here is to treat return, continue, and (I'm guessing) break as expressions instead of statements. This would let you embed them in other expressions and we would extend our existing flow analysis to take that into account for type promotion.

We already did the same thing many years ago for throw. It used to be a statement (as it is in most other languages), and we made it into an expression in order to support code like:

someShortMethod() => throw 'Unimplemented.';

I don't think there's anything technically problematic about making this change. I do have a lot of concerns about readability and whether it is worth the complexity of changing it. Many readers have a strong preference that control flow is easy to notice and generally appears at the beginning of a line. This would reduce that significantly.

@munificent
Copy link
Member

Conditional return, continue, break are very common operations. Formatter leaves them on the same line
if (x > 1) return;

This is true, and while many users like that feature, some definitely do not.

@Levi-Lesches
Copy link

I agree with the sentiment -- it's not like having the extra line of code impacts product performance for the end-user. Separating the control flow out to its own line makes it clear what's happening, and aside from the occasional if (cond) return;, I think that's probably the best way to do it.

@dart-lang dart-lang deleted a comment from ykmnkmi Dec 15, 2021
@lrhn
Copy link
Member

lrhn commented Dec 16, 2021

The parseIntTuple is not what I'd consider readable.
I had to think far too hard to follow the logic, and I kept wondering why the char didn't return a bool, so it would at least use || return null; instead of ?? return null;.

It requires a completely new way of thinking about the ?? operator (for me, others might differ).
When I see e1 ?? e2, I read "the value of e1, or if e1 is null, the value of e2, which is probably not null".
Here, seeing the ?? return null, I had to go back and think "if e1 is null then ...." which is technically equivalent, but very, very different in my mind's pattern recognition.

I'd probably write it as:

IntTuple? parseIntTuple(ParserContext context) {
  int? left, right;
  if (context.char($lparen) == null ||
      (left = context.parse('left integer', parseInt)) == null ||
      context.char($comma) == null ||
      (right = context.parse('right integer', parseInt)) == null ||
      context.char($rparen) == null) {
    return null;
  }
  return IntTuple(left!, right!);
}

But that's just that example.
I'm not personally opposed to allowing control flow expressions. We allowed throw, and nobody has been complaining about that. They trivially work as expressions since they have type Never, so assigning them a value is not necessary
(unlike expression switch or loops).

@jakemac53
Copy link
Contributor

jakemac53 commented Dec 16, 2021

Fwiw I prefer the original parseIntTuple with the proposed feature 👍 . I find it intuitive personally. And I very much dislike the latter example assigning to vars inside a big if like that, it just screams of code golf to me lol.

This is how I think most would write this today, very explicit and easy to follow and no nesting:

IntTuple? parseIntTuple(ParserContext context) {
  if (context.char($lparen) == null) return null;
  var left = context.parse('left integer', parseInt);
  if (left == null) return null;
  if (context.char($comma) == null) return null;
  var right = context.parse('right integer', parseInt);
  if (right == null) return null;
  if (context.char($rparen) == null) return null;

  return IntTuple(left, right);
}

@rakudrama
Copy link
Member

rakudrama commented Dec 16, 2021

The precedence of throw as an expression is unfortunate, as it does not allow

  x >= 0 || throw ArgumentError.value(x);

I think this would nice because the checked precondition is written directly, the same as in an assert.

To get this to work one has to use parentheses, with makes it less attractive:

  x >= 0 || (throw ArgumentError.value(x));

The conventional alternative of using an if-statement requires one to negate the precondition.
This is a mental tax I would prefer to avoid.

  if (x < 0) throw ArgumentError.value(x);

or

  if (!(x >= 0)) throw ArgumentError.value(x);

Like the OP, if I was to change the method using ... || throw ... to return null instead of throwing, I would want return on the same footing as throw.

I hope the precedence problem of throw can be fixed whether or not we do allow returning from the middle of an expression.

@ykmnkmi
Copy link

ykmnkmi commented Dec 17, 2021

@rakudrama this is right way to do:

if (x < 0) {
  throw ArgumentError.value(x);
}

for me, the fact that throw can be used in expressions is not acceptable, even this:

Never error() => throw Error();

@eernstg
Copy link
Member

eernstg commented Feb 7, 2022

I agree with @munificent that it might not be a big problem technically to support return/break/continue/rethrow as expressions. I spelled out the details a bit more in issue #2099, and then discovered that there's a lot of overlap with this one, so I closed #2099 as a duplicate, and put the details in this comment:

So the point is that we already have the ability to abruptly terminate an ongoing expression evaluation using throw e, but in some situations it can be convenient to complete the expression evaluation 'returning' rather than 'throwing', or 'continuing', or 'breaking', and we could introduce <returnExpression>, <breakExpression>, <continueExpression>, <rethrowExpression>, and allow them to occur as expressions of type Never, with the same syntactic precedence as <throwExpression>.

The semantics would be to immediately terminate the evaluation of the enclosing expression, and then give rise to the same data transfer and control transfer as the corresponding <returnStatement>, <breakStatement>, etc., do today.

This proposal may seem quite disruptive at the syntactic level, but an experimental change to Dart.g seems to support the assumption that we can simply do the following:

returnExpression // Updated, was `returnStatement`.
    :    RETURN expression?
    ;

breakExpression // Updated, was `breakStatement`.
    :    BREAK identifier?
    ;

continueExpression // Updated, was `continueStatement`.
    :    CONTINUE identifier?
    ;

rethrowExpression // Updated, was `rethrowStatement`.
    :    RETHROW
    ;

// Delete `returnStatement`, `breakStatement`, `continueStatement`, 
// `rethrowStatement` from `nonLabelledStatement`, they are now
// covered by `expressionStatement`.

throwExpression // Should be renamed, e.g., `abruptCompleteExpression`.
    :    THROW expression
    |    breakExpression
    |    continueExpression
    |    returnExpression
    |    rethrowExpression
    ;

A grammar update with some improvements beyond the ones shown above can be found at https://dart-review.googlesource.com/c/sdk/+/231949.

@johnniwinther, @scheglov, @mkustermann, @sigmundch, how disruptive does this feature seem to be from your point of view, for the static analysis, code generation, or execution of programs? [sorry about the duplicate question, I just asked the same question in #2099, but #2099 has been closed as a duplicate of this issue.]

My guess is that the static analysis can just give the new expressions the type Never, and almost everything done for throw expressions could then be reused with these new expressions.

But during code generation and execution there might be local state (say, a non-empty expression evaluation stack) that calls for further operations before jumping, and I have no idea how hard it would be to handle that, or how much of the existing functionality concerned with throw expressions could be reused with the new expressions.

@eernstg
Copy link
Member

eernstg commented Feb 7, 2022

@rakudrama wrote:

The precedence of throw as an expression is unfortunate

We had that discussion a while ago (but I can't find the relevant issue right now). One possible remedy was

ifNullExpression
    :    logicalOrExpression 
         (('??' logicalOrExpression)* ('??' expressionWithoutCascade))?
    ;

This would allow throw e as well as return/return e, break/break L and continue/continue L after the last ??.

This update is included in the grammar update in https://dart-review.googlesource.com/c/sdk/+/231949.

@lrhn
Copy link
Member

lrhn commented Feb 7, 2022

The problem with the precedence of throw is that it looks like a prefix operator, but if it was, then throw 2 + 3 would mean (throw 2) + 3. And that's meaningless because you never want to call a method on Never.

So, instead it means throw (2 + 3), but to make precedence consistent, that also means that you can't do 1 + throw 2.

Basically, a control flow should always be in tail position in an expression, because you shouldn't do anything after a control flow operation. Using precedence to determine "tail position" isn't really working for us.
(Also, it's not entirely true that it's just about being in tail position, since a potentially throwing expression in argument position is fine, foo(bar ?? throw "NULL!"). A definitely throwing expression in argument position is useless, so we need the concept of definitely escaping and potentially escaping to properly prevent misuse, and then it's far beyond what we can do with grammar.)

@eernstg
Copy link
Member

eernstg commented Feb 7, 2022

@lrhn wrote:

The problem with the precedence of throw ...

I think it will be a useful improvement to allow e1 ?? throw e2 and tolerate that all the other expressions that could contain one of these new expressions, say e1 + (throw e2), don't get rid of these parentheses. The ?? expression could be common enough and already readable enough to work just fine, but for all the others6 I think the parentheses are actually useful. For instance, a + throw b + c comes across as less obvious than either a + (throw b) + c or a + (throw b + c). Also, as you mentioned, + is not conditional (and this is also true for most other binary expressions), and this means that e1 + throw e2 is a lot less useful than e1 ?? throw e2.

This should serve to motivate the not-so-impressive generality of the ifNullExpression adjustment I suggested here: It handles ??, and does nothing about any other binary expression.

@eernstg
Copy link
Member

eernstg commented Feb 7, 2022

About the readability issue (which seems to be the main argument against this feature): I think we should put the main emphasis on useful and readable forms like

final myField = this.myField ?? return;

and not worry so much about the ability to write obscure code. Honestly, we can write obscure code with any language mechanism. ;-)

@stereotype441
Copy link
Member

@lrhn said:

Basically, a control flow should always be in tail position in an expression, because you shouldn't do anything after a control flow operation. Using precedence to determine "tail position" isn't really working for us. (Also, it's not entirely true that it's just about being in tail position, since a potentially throwing expression in argument position is fine, foo(bar ?? throw "NULL!"). A definitely throwing expression in argument position is useless, so we need the concept of definitely escaping and potentially escaping to properly prevent misuse, and then it's far beyond what we can do with grammar.)

From my personal experience, a definitely throwing expression in argument position (or some other non-tail position) is not actually useless. I often find it really helpful, when making changes to code I'm less familiar with, to replace various critical expressions with a temporary throw 'TODO', then run unit tests to see what fails, then isolate that test and run it under the debugger, pausing at the site of the "throw" expression to inspect local variables and figure out what I want the behavior to be.

When I do this, the analyzer's "dead code" hint (which is based on flow analysis) does a pretty good job of demonstrating the absurdity of my temporary code, which prevents me from accidentally commiting it to source control. So I actually think we are doing pretty well already at preventing misuse with our existing diagnostics.

@lrhn
Copy link
Member

lrhn commented Feb 7, 2022

That still means allowing all the useful and readable forms, because otherwise it's surprising when something that looks useful and readable doesn't work.
So, what are those?

I'm guessing anything that might shortcircuit to avoid the control flow, because an unconditional control flow operation doesn't need to be inside an expression.

  • expr ?? return
  • expr || return
  • expr && return
  • expr ? (expr|return) : (expr|return) (but not both returns? Then it would count as a return itself.)

Which brings up the example e1 || return e2 && e3. If I write it as (e1 || return e2) && e3, then it makes sense.
Without the parentheses, it's much less readable.
As I interpret the grammar suggestion above, this e1 || return e2 expression would not be allowed to be followed by && e3, not unless it's parenthesized. Right?

(I don't think ??=/&&=/||= are worth special-casing, since they'll (1) already work, and (2) will never do the assignment if the RHS definitely escapes.)

@eernstg
Copy link
Member

eernstg commented Feb 8, 2022

e1 || return e2 && e3

That would mean e1 || (return e2 && e3). In general, the use of expression and expressionWithoutCascade in <returnExpression> makes the parsing greedy at this point, in the sense that it will include almost anything that amounts to an expression in the lookahead.

I'd suggest that we allow all the constructs that follow naturally from the grammar (so there wouldn't be any special rules about syntactic contexts where a <returnExpression> could occur), and then we leave it to the 'dead code' analysis to flag the situations where the control flow seems to be meaningless.

@lrhn
Copy link
Member

lrhn commented Feb 8, 2022

My bad, I meant e1 && return e2 || e3. If return still greedily captures everything, then that will mean e1 && (return e2 || e3), where e1 && e2 || e3 means (e1 && e2) || e3.
I fear that might be surprising — but if we teach that "return is always greedy", it might be predictable anyway (and it's unlikely to be a common occurrence).

@eernstg
Copy link
Member

eernstg commented Feb 8, 2022

Yep, I think

"return is always greedy"

is a rather natural rule: Anybody who wants to do anything other than "take it all" will need the parentheses, and that will be once in a blue moon anyway, and we'll probably benefit from having those parentheses when reading code simply because it is going to be tricky stuff when they are needed.

We might need to note that we're relying on the disambiguation that the Dart grammar has always relied on: The first rule that yields a successful parse is the one that we will use (which is what top-down parsers will naturally do anyway), so this is cementing an already deep property of syntactic disambiguation in Dart. However, anyone who attempts to write a Dart grammar for yacc or any LR/LALR parser generator will immediately notice that there is a lot of work to do in order to eliminate shift/reduce and reduce/reduce conflicts. So this isn't new, it's just one more step in a direction where we have already taken many steps.

@stereotype441
Copy link
Member

I like "return is always greedy" too. And there is even a precedent for it: expression function bodies have always been greedy in precisely this way, so for example e1 * () => e2 + e3 means e1 * (() => e2 + e3), whereas e1 * e2 + e3 means (e1 * e2) + e3.

Personally I would be comfortable generalizing the rule to apply to throw as well as return, which would make expressions like e1 || throw e2 possible. I believe it would be a non-breaking change, since currently we forbid throw from appearing in higher-precedence contexts.

@scheglov
Copy link
Contributor

scheglov commented Feb 8, 2022

From the static analysis POV I don't see any issue with adding these new expressions.

@munificent
Copy link
Member

Personally I would be comfortable generalizing the rule to apply to throw as well as return, which would make expressions like e1 || throw e2 possible.

Yeah, I definitely think that if we do this, throw and return should be parsed the same way.

If we wanted to really open the can of worms... it would be great if await worked this way too.

@rakudrama
Copy link
Member

I think making return, break and continue be general expressions would be a mistake. In almost all places they are not meaningful, but would enable keyword salad like return throw return continue; to be a valid statement that only later generates a lint or warning. We lose something with the generality.

One thing we lose is parser recovery. Currently, break is a statement. If break is valid in any expression context, it becomes a much weaker parser synchonisation signal. "break is a statement, except when preceded by ??" would be weaker that "break is a statement", but still valuable. "break can occur almost anywhere" impairs parser recovery.

I really like this example (and the throw/break/continue variants), since the named value is non-nullable:

final myField = this.myField ?? return;

I also like e1 || return e2 at the statement level as explained below.

I'm not sure the other cases carry their weight.

Usefulness

e1 && return e2 and e1 || return e2 always evaluate to false and true. Evaluating to a fixed value makes these expressions of limited use in an expression context, so they would tend to be used predominantly in a statement context.

In a statement context

e1 && return e2;

is pretty much the same as

if (e1) return e2;

Not worth it.

In contrast, I am quite taken with e1 || return e2; at the statement level:

0 <= index && index < list.length || return null;

This is almost the perfect minimal code. It expresses the conditions which are true after the statement, and what happens when it is not true. It establishes an invariant that can be used to reason about the following code. The guard is exactly what you would write in an assert statement, making it easy to move between asserts and code that has to do checks. Clear, concise and malleable.

There are alternatives, but they all feel a little heavier:

// Python-like `unless`
unless (0 <= index && index < list.length) return null;

// if-not expression
if! (0 <= index && index < list.length) return null;

// infix `else` statement
0 <= index && index < list.length else return null;

// An new infix statement operator `|||`
0 <= index && index < list.length ||| return null;

Parsing

For operators &&, ||, ??, the expression (e1 <op> return e2) <op> e3 is useless, since a value of e1 that short-circuits the return will also short-circuit e3. e3 is dead code. There are lots of similar useless combinations.

"return is always greedy" seems useful in converting expressions that contain dead code into expressions that do something useful. What worries me is that it treats return and break inconsistently. e1 && break || e3 yields the value of e3, but e1 && return e2 || e3, when greedily parsed as e1 && return (e2 || e3), yields false and does not evaluate e3.

Greedy parsing of function expressions has worked well, but I'm not sure the lesson translates. Greedy parsing of expression functions works well only because we have no operators on function. Consider if we added a function composition operator + such that (f + g)(x) == g(f(x)). Parsing expression functions would now be much more perilous.

Rather than making return an expression, we could permit return only after ?? and ||. Conceptually, we are adding a family of two-token operators, ifNullReturn (?? return), ifFalseBreak (|| break), etc. It is never necessary to parenthesize return because we can always parenthesize the return expression.

The grammar change that incorporates this limited 'tail position' element looks like

ifNullExpression
    :    logicalOrExpression
         ('??' logicalOrExpression)* ('??' ifNullDiversion)?
    ;
ifNullDiversion : RETURN logicalOrExpression? ;
ifNullDiversion : THROW logicalOrExpression ;
ifNullDiversion : BREAK identifier? ;
ifNullDiversion : CONTINUE identifier? ;

logicalOrExpression
    :    logicalAndExpression
         ('||' logicalAndExpression)* ('||' logicalOrDiversion)
logicalOrDiversion : RETURN logicalAndExpression? ;
logicalOrDiversion : THROW logicalAndExpression ;
logicalOrDiversion : BREAK identifier? ;
logicalOrDiversion : CONTINUE identifier? ;

"return is always greedy" is a small change:

ifNullDiversion : RETURN expression? ;
logicalOrDiversion : RETURN expression? ;

@ykmnkmi
Copy link

ykmnkmi commented Feb 9, 2022

if that's would be accepted, is it also possible to accept try without catch/finally as an expression that returns null on failure?

int value = try int.parse(input) ?? return;

@mateusfccp
Copy link
Contributor

if that's would be accepted, is it also possible to accept try without catch/finally as an expression that returns null on failure?

int value = try int.parse(input) ?? return;

This would be expressed as

int value = int.tryParse(input) ?? return;

@ykmnkmi
Copy link

ykmnkmi commented Feb 9, 2022

@mateusfccp of course, it's for example.

@lrhn
Copy link
Member

lrhn commented Feb 9, 2022

I'm not particularly smitten with condition || return 42; as a statement. I still prefer if (!condition) return 42;.

The places I see a use for control flow is inside larger expressions, not necessarily at the end.

Something like return ((x = foo[0]) ?? return false) + ((y = foo[1]) ?? return false) < 5; where you bail out early in some cases. A more readable version would be:

var x = foo[0] ?? return false;
var y = foo[1] ?? return false;
return x + y < 5;

Again, it's an early bailout in the case where you don't have a value. For booleans, it should be when you don't have a property needed for the following computation.
Like if ((x > 0 || return) && (y > 0 || return)) doSomethingWith(x * y);.
Still, probably more readable as

if (x <= 0 || y <= 0) return;
doSomethingWith(x * y);

As @stereotype441 pointed out, the e1 || return and e2 && return are known to have a fixed value if they don't return.

I definitely do want e1 ? e2 : return and e1 ? return : e2 as options.

We also need to restrict such control flow to places where it makes sense (inside function bodies).
We have expressions outside of function bodies in a number of places:

  • static, library or top level variable initializers.
  • parameter default values.
  • initializer list expressions (including asserts).

We might also need to disallow control flow expressions in late local variable initializers. Or not, the access to the variable needs to happen in the same scope, so it's probably possible to execute the side effects. It's just not very readable that a variable reference like x will return.
(Or we should simply drop late local variables having initializers again, they're too weird with their out-of-order evaluation.)

@ykmnkmi I've considered expression level exception handling, but haven't found a good model.
It's annoying to have both int.parse and int.tryParse which only differ in how they report errors. It's very convenient to use null to represent a failure, but it's also not really the right thing to do. So, it would be nice if we could get a similar convenience for throws, which the current try/catch statement makes very verbose.

Catching any error and turning it into null is one possibility, but .... catching any throw, and treating them all the same, is very error-prone (it can hide unexpected errors). So I'd definitely want an on clause.
I'm not opposed to try e1 on E catch (error, stack) e2. But hen I can't find a good argument against allowing a finally block, though, but try e1 finally { statements; } is a weird way to introduce statements into expressions. (But it works, since the finally block doesn't need to have a value.)

I'd also accept e1 catch (E e) e2. (It's not Dart 1 any more, I have no problem changing the try/catch to allow the type in the catch clause instead of in a separate on clause.) Since catch is a keyword, that should likely be parsable.

@lrhn
Copy link
Member

lrhn commented Feb 9, 2022

The "small change" of ifNullDiversion : RETURN expression? ; is ambiguous.
Something like e1 || return e2 == e3 would be parseable in two ways because e1 || return e2 is a valid logicalOrExpression.
I we rely on rule precedence in the parsing, it might turn out correctly - I really can't begin to predict whether it does or not.

The alternative would be rules like

logicalOrExpression
    :    (logicalAndExpressionNoDiversion '||')* logicalAndExpression
    |    (logicalAndExpressionNoDiversion '||')* logicalAndExpressionNoDiversion '||' diversion

logicalOrExpressionNoDiversion
    :    (logicalAndExpressionNoDiversion '||')* logicalAndExpressionNoDiversion

diversion ::= 
    RETURN expression? 
  | THROW logicalAndExpression 
  | BREAK identifier? 
  | CONTINUE identifier? 

(all the way down the grammar hierarchy) which ensures that the diversion can only be at the end of an expression, so nothing can occur after a diversion in an expression.

@eernstg
Copy link
Member

eernstg commented Feb 9, 2022

@rakudrama wrote:

making return, break and continue be general expressions
would be a mistake .. keyword salad

I certainly think it's worth considering a minimal model where only a few constructs allow for the new expressions that introduce non-trivial control flow, as you propose here.

However, we would presumably also be able to hint/lint the keyword salad, because it gives rise to something that is similar to "unreachable code"? It would be more like "non-normal completion can never occur" if we have return return 1, because the outer return can't actually return, but the point is that we can recognize and flag situations where the core semantics of a construct can never be executed, so that construct should probably not exist (like the outer return in return return 1)). Of course, it could also flag return throw 1;, and that is probably OK.

I understand that this is extra work, but the overall outcome could be an improvement.

e1 && return e2 and e1 || return e2 always evaluate to false and true.

Or they give rise to a non-normal completion ('returning with an object'). How do we know that this is never useful? We often try to avoid introducing arbitrary restrictions on any given language construct, because there might be legitimate and helpful usages that we haven't considered.

Car? getElectricCar(Customer customer) {
  // Compute number of electric cars that we can produce today, not already sold.
  int eCarCount = ...;
  var car = Car(
    isElectric: eCarCount > 0 || return null,
    isHybrid: customer.wantsHybrid() && return null,
    ... // Other constructor arguments.
  );
  ... // Register `car` as sold, count down available materials.
  return car;
}

So the idea is that we can bail out of arbitrary computations. We might then use return/return e (bailing out of the entire function), or we might break (bailing out more locally inside a function) or continue (bailing out of an attempt, presumably in order to try something else).

Obviously, it is possible to write completely unreadable code using these constructs, just like it's possible to write unreadable code using any other construct, but if we as a community can foster a set of useful idioms using the new constructs then I believe it can be used in a readable manner, and I would tend to assume that it's better to avoid arbitrary restrictions.

The approach that I mentioned here would provide the more general syntax with ??, and it could also be used with || and &&.

@lrhn
Copy link
Member

lrhn commented Feb 9, 2022

We could introduce a rule of "don't use a value of type Never", like we have for void.
(Never is the dual of dynamic for member access, and the dual of void for using the value. Most peculiar. 😁 )

Any use of a Never-typed expression will be dead code, so it's not like we prevent anything useful.
You can do var x = condition ? something : throw bad; because you are not using some of type Never, but of type LUB(typeof(something), Never) == typeof(something). Unless something has type Never too, then the assignment is dead code.

That would prevent return return x as well as the currently allowed throw throw x.

That also automatically restricts use of Never expression to conditional sub-expressions (operands of a short-circuiting operator or branches of a conditional expression) or to tail-position of expression statements (which are then terminating for the block). Basically, you can only use Never where the "continuation" dominated by that expression is empty.

@stereotype441
Copy link
Member

We could introduce a rule of "don't use a value of type Never", like we have for void.

Wouldn't this also prevent expressions like e1 ?? return, which was the original request?

@lrhn
Copy link
Member

lrhn commented Feb 9, 2022

No, the static type of e1 ?? return is NONNULL(typeof(e1)), not Never. It's fine to use that.

The tricky part here is to define what it means to "use" a value. Basically, it's having the value assigned to anything (RHS of assignment, parameter of invocation, operand of non-short-circuiting operator), or being used as receiver (of method or operator, which implicitly assigns the value to this).

Here return is in tail-position in e1 ?? return, it's not passed to any operator which sees its value, instead the short-circuiting will discard the first operand entirely and just use the value of the second instead. So it can have type Never.
The expression e1 ?? return itself does not have type Never, so it's not a problem.

Any operation where the value of the expression can either be found without evaluating one operand, or will be the value of that operand, effectively puts the operand in "tail position". That's all the short-circuiting operators and ?/:, parenthesized expression, and ... probably nothing else. An expression of type Never which isn't in that position, and is not the expression of an expression statement, is probably introducing dead code.

@rakudrama
Copy link
Member

rakudrama commented Feb 9, 2022

The "small change" of ifNullDiversion : RETURN expression? ; is ambiguous.

Only if you disagree with @eernstg that the grammar is already ambiguous and conflicts are resolved by being greedy. I was taking that as a premise.

@rakudrama
Copy link
Member

I think it is a bad idea to make e.g. + return a valid pair of tokens and rely on dead code warnings.

Consider when you are in the editor and typing a statement just before an existing return-statement:

x = z +return;

The analyzer has to make sense of that partial state which is now a valid expression. Should a completion be offered?
The function now parses, so should we do further analysis and complain to the user about dead code?
If a diversion is valid only after, say, ??, it is clear that the user has not finished editing the method.

/cc @bwilkerson

@eernstg
Copy link
Member

eernstg commented Feb 9, 2022

I think it is a bad idea to make e.g. + return a valid pair of tokens

Right, it is unlikely to be useful to use the new 'non-normal completion' expressions as operands of operators that do not have a conditional element in their semantics, except of course for expression statements. So the immediate candidates are the following:

  • As the expression e in an expression statement e;.
  • Right operand of ??, ||, &&.
  • Right operand of ??=, (and ||= and &&=, if introduced).
  • As the expression e2 in e1?[e2].
  • <ifElement>: As the "then" element or the else element.
  • <forElement>: In the <forLoopParts>, as in a for-statement, and as the body element.
  • In <forLoopParts>, only with C-style for-statement/element (init; e; es): As an element in the expression list es.
  • In an <assertStatement>, as the second argument.

Several of these may look silly (e.g., a <forElement> like for (var v in iter) break would be more readable if written as if (iter.isNotEmpty) break), but it's usually not a safe bet that we have already noticed every possible usage which is meaningful and useful, so it's at least a good idea to look at the list, and perhaps add an extra element if I forgot something. Then we can return to ??/||/&& only, if we're convinced that's enough.

By the way, + return would not be a valid sequence of tokens with my proposal anyway, just like + throw won't occur in a syntactically correct program today, because <throwExpression> (and hence the new ones) derive directly from <expression> and from <expressionWithoutCascade>, and the right operand of + is a <multiplicativeExpression>. ;-)

@Jetz72
Copy link

Jetz72 commented Feb 10, 2022

By the way, + return would not be a valid sequence of tokens with my proposal anyway, just like + throw won't occur in a syntactically correct program today, because <throwExpression> (and hence the new ones) derive directly from <expression> and from <expressionWithoutCascade>, and the right operand of + is a <multiplicativeExpression>

Not sure if this is fundamentally different, but it can be noted that it is currently valid to write + (throw "foo") if parentheses are used. Can also use it as a parameter to a function e.g. int.parse(throw "car");. Doesn't seem useful to directly use a Never expression as a parameter and it should probably mark the surrounding expression as dead code, but at least it's less likely to come up unintentionally if the control flow expression has to be surrounded like that.

@mattrberry
Copy link

I think this feature would be particularly welcome now with the introduction of switch expressions. I'd argue that allowing control-flow in switch expressions is arguably an easier to read solution that what we have today. For example:

Node? someFunc(Node node) {
  final err = switch(node) {
    case Valid(cond: bool b) when b => return node;
    case HasErr(err: String err) => err,
    _ => 'default error',
  };
  handleErr(err);
  return null;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests