[SR-11711] [Parse] Single-expression implicit returns within #if declarations

[maustinstar]

Summary

Implicit returns for single line expressions do not currently allow for omitting "return" in single-expression #if declarations.

Problem

From SR-11711:

var z: Int {
    #if false
    0
    #else
    1 // error: missing return in a function expected to return 'Int'; did you mean to return the last expression?
    #endif
}

It would be a really useful and clean idiom to use with platform tests, were it legal

Impact

This PR enables a new set of implicit return declarations to be considered valid.

After merging this PR:

var z: Int {
    #if false
    0
    #else
    1
    #endif
}

This is now valid, as is every test case covered by test/Parse/omit_return.swift, test/expr/closure/single_expr.swift, and others when using #if declarations. Near-verbatim files test/Parse/omit_return_ifdecl.swift and test/expr/closure/single_expr_ifdecl.swift test the same cases wrapped in #if declarations.

For closures, the implicit result type is now inferred from the single-expression in the active clause.

var int_or_string = {
    #if FLAG
    42
    #else 
    "foo"
    #endif
}

This now resolves to () -> Int or () -> String depending on the flag. (This previously resolved to () -> () with an unused literal).

Limitations

The only scenario that can't be replicated (yet) is this:

var someZero: Int? {
    .some(0)
}

var someZero_ifdecl: Int? {
    #if true
    .some(0) //  error: unexpected platform condition (expected 'os', 'arch', or 'swift')
    #endif
}

This seems to be a problem with the way #if declarations are lexed, and probably warrants a separate pull request. For now, syntax like this works:

var someZero_ifdecl: Int? {
    #if true
    Int.some(0) //  no error
    #endif
}

Implementation Notes

Using back node for single-expression

Previously, when getting or setting the single-line expression, it was common to access the front node. This implementation now retrieves the back node. This will retrieve the same node for single-expression bodies, but #if declarations that look like single expressions actually contain two nodes: the if-declaration, and the active clause. To get the active clause in these scenarios, we need to use the back node.

Unsupported if-declaration bodies

Consider the following cases:

func foo() -> Int {
  #if true
  #endif
  1
}

func bar() -> Int {
  #if false
  ...
  #endif
  1
  #if false
  ...
  #endif
}

Such cases are intentionally not supported. While the flag condition combinations seem to leave a single-expression, certain flag combinations could result in cases with an unused literal or an error for missing a return statement. Additionally, any flag combination state can already be represented within a single #if declaration.

Links

Resolves SR-11711 and SR-3417.

Tagging @nate-chandler for authoring #23251 and @dabrahams for reporting SR-1171.

[xedin]

Active condition reliance makes me nervous because it means that new behavior would be inconsistent between static and purely dynamic cases which is not great. This might be worth discussing on forums before merging. I'm also going to add @rintaro to take a look.

[maustinstar]

Thanks for looking. The reason I singled out the active clause is because something like this is considered valid today:

var z: Int {
    #if false
    0
    #else
    return 1
    #endif
}

Can you give an example of dynamic cases? From my understanding, only the active clause is ever executed.

[xedin]

Maybe something like #if swift(5.2) or similar where there is no way to find that statically because -swift-version is a compiler flag.

[maustinstar]

I just ran a local test on my machine using this snippet:

var z: Int {
    #if swift(>=5.2)
    0
    #else
    1
    #endif
}

print(z)

It prints 0, and switching the condition to swift(<5.2) prints 1. From my understanding, at the point where I check for implicit returns, the active clause has already been determined by the compiler. Is your concern more about code completion? Should I add tests to cover such dynamic scenarios?

[rintaro]

Could you add more test cases for closures? All I could find are

          {
              #if true
              print("howdy")
              #endif
          }

in () -> Void context, or some non-single-expression closures. Also, please add some #else test cases.

For example:

let fn = {
  #if FLAG
  "foo"
  #else
  42
  #endif
}

We want to make sure that fn is resolved to () -> String or () -> Int depending on the FLAG conditional compilation flag.

[rintaro]

FYI this should resolve https://bugs.swift.org/browse/SR-3417 too

[xedin]

@maustinstar Maybe I misunderstand the feature but it sounds like it's not always possible to determine active case statically, so it would be great to get some more tests for that and/or adjust the behavior to check that all of the cases have a single expression inside and if not point out locations where explicit return is expected for e.g.

#if <flag>
  print("")
#else 
  let x = 1 + 2
  x
#endif

would type-check if we needed return to x.

[maustinstar]

Adding test cases as @rintaro suggested.

@xedin, I see your point. I also want to mention that code like this is valid in the current version of Swift (even before this pr):

var z: Int {
    #if FLAG
    return 0
    #else
    var x = 1 + 2
    x
    #endif
}

When your build setting sets the compiler flag, FLAG, to true, this compiles successfully. When false, the missing return has an error. This is because the non-active clause is not checked for a return statement during compilation.

I get that this behavior doesn't seem expected, but because it is the current behavior of Swift, it seems out of scope for this PR, and might better fit in a bugfix by itself. What do you think?

[xedin]

Ok, if that's the way it works today - I'd say let's just keep it...

[maustinstar]

Okay thanks for the chat. I'll also write a new SR about that behavior and see if others think it's a bug.

[maustinstar]

Okay @rintaro, I made changes to ParseExpr.cpp to support the same functionality for closures. I also created the file test/expr/closure/single_expr_ifdecl.swift which copies tests from test/expr/closure/single_expr.swift and adds a test case like the one you mentioned at the bottom of the file.

[maustinstar]

@rintaro thank you for the reviews! I've added the assertion

[xedin]

@swift-ci please test

[xedin]

@swift-ci please test source compatibility

[rintaro]

@swift-ci Please test Windows

[xedin]

Thank you, @maustinstar! Please don't forget to resolve related SRs.

[maustinstar]

Resolved! Thank you again @xedin and @rintaro!