Disallowing unreachable code

Here's a different principle: When Swift generates a warning, the mistake has no runtime effect on the behavior of the code, but things like non-exhaustive `switch` or a missing `try` represent code where you haven't told the compiler how to handle some of the circumstances it might encounter.

If you stick a `return` in the middle of some code, the compiler knows exactly how to interpret that: Return there and don't run the code after it. If you write a non-exhaustive `switch`, what is the compiler supposed to do? Fall out of `switch` statement? Trap? It's unclear. Similarly, if there's no `try`—particularly outside of a `do`/`catch` block or `throws` function—what behavior should the compiler assume you want?

Trapping is clearly the proper choice, because Swift's philosophy is to trap whenever the program finds itself in a state that the programmer did not anticipate. And yet you don't want the *lack* of something to cause a trap; you should at least be able to see the operation so you have a chance to recognize the potential to trap. (Implicitly unwrapped optionals are the exception that proves the rule--many programmers ban them because they cause invisible trapping. You don't want people banning `switch` or `throw` statements because of their invisible danger.) Any other option, though, runs the risk of letting a program run off the rails and do the wrong thing.

So we emit an error in these cases because the programmer has written ambiguous code, and the compiler has no good option for resolving the ambiguity. On the other hand, if you use `var` instead of `let`, or write code after a `return`, the instructions your code is giving the compiler are unambiguous; they're just not phrased as well as they could be. So we emit a program that does what your code says and politely point out that you could write it better.

···

On Mar 28, 2017, at 9:55 PM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

Missing 'try' is a fatal issue?

That could be argued I suppose, I was referring to unreachable code, unused variables,
variables that are never mutated, etc.

And what about non-exhaustive switch?

Both of these existing rules seem to violate the principle claimed, because they are hazards to incomplete or experimental changes that might lead people to use quick fixes (try!; default) that are not associated with a warning, whereas a warning instead of the error would (as you claim) signal to the user there are pending fixes before commit.

--
Brent Royal-Gordon
Architechies

I think the idea is that it also adds a warning so you can find it later.

···

On Mar 29, 2017, at 6:06 AM, Alex Blewitt <alblue@apple.com> wrote:

On 29 Mar 2017, at 11:38, Jonathan Hull via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Mar 27, 2017, at 11:27 AM, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

In fact, we should probably double-down on this design and add an "unimplemented" expression that always triggers warnings and just traps if you try to evaluate it. That expression would be a logical thing to use in e.g. code snippets automatically inserted by an IDE.

Yes, please.

You can do this already:

func unimplemented<T>(_ file:String = #file,_ line:Int = #line) -> T {
  fatalError("Not implemented \(file):\(line)")
}

let f: String = unimplemented()

Non-exhaustive switch is like a missing return statement: the code isn't just "suspicious", it's potentially unsound if the warning is correct. That's not true of unreachable code or ignored return values. We could, of course, invent semantics for it, like implicitly asserting or implicitly falling through; but the former is inconsistent with our safety goals, and the second is likely to just lead to worse downstream diagnostics.

You're correct that enforcing "try" as a hard error is not consistent with being lax about experimental code, but the entire point of "try" is to reliably mark places that can throw; making it only a warning would completely undermine that. If we had a language mode focused on experimental code, then yes, it would be appropriate to disable the "try" diagnostic; but adding a language mode like that would be really destructive long-term, outside of specific situations like coding in the debugger.

I would say that the "incomplete" side of the coin is more important to us than the "experimental" side. We don't want laxer rules for experimental code because in practice there's usually no bright line between prototypes and production. But if our hand isn't being forced, and we don't have strong language-design reasons to enforce the rule, a warning is fine.

John.

···

On Mar 29, 2017, at 12:55 AM, Peter Dillinger <Peter.Dillinger@synopsys.com> wrote:

On Mar 28, 2017, at 9:40 PM, Peter Dillinger <Peter.Dillinger@synopsys.com> wrote:

Agreed, we have the right design here. The go community has shown the result of taking
a hard line on this, and it really hurts refactoring and other experimental “pound out some
code” prototyping use cases. We use warnings for things that “should be cleaned up before
code is committed”, but which is not itself a fatal issue.

Missing 'try' is a fatal issue?

That could be argued I suppose, I was referring to unreachable code, unused variables,
variables that are never mutated, etc.

And what about non-exhaustive switch?

Both of these existing rules seem to violate the principle claimed, because they are hazards to incomplete or experimental changes that might lead people to use quick fixes (try!; default) that are not associated with a warning, whereas a warning instead of the error would (as you claim) signal to the user there are pending fixes before commit.

In theory, your position seems defensible, but I'm not seeing consistency in application.

@available(*, deprecated, message: "Don't forget to implement this")
func unimplemented<T>(_ file:String = #file,_ line:Int = #line) -> T {
  fatalError("Not implemented \(file):\(line)")
}

let f: String = unimplemented()

···

On 29 Mar 2017, at 14:10, Jonathan Hull <jhull@gbis.com> wrote:

I think the idea is that it also adds a warning so you can find it later.

I was suggesting that it would be a useful addition to the language, not that it
necessarily needed new compiler support.

Does someone want to write a proposal for it?

John.

···

On Mar 29, 2017, at 9:15 AM, Alex Blewitt <alblue@apple.com> wrote:

On 29 Mar 2017, at 14:10, Jonathan Hull <jhull@gbis.com <mailto:jhull@gbis.com>> wrote:

I think the idea is that it also adds a warning so you can find it later.

@available(*, deprecated, message: "Don't forget to implement this")
func unimplemented<T>(_ file:String = #file,_ line:Int = #line) -> T {
  fatalError("Not implemented \(file):\(line)")
}

let f: String = unimplemented()

Personally, what I'd like to see is for the existing <#whatever#> placeholder syntax to be treated as an unimplemented() call. That probably *would* require compiler support, although fortunately we already parse this syntax into an EditorPlaceholderExpr.

···

On Mar 29, 2017, at 8:11 AM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

I was suggesting that it would be a useful addition to the language, not that it
necessarily needed new compiler support.

--
Brent Royal-Gordon
Architechies

Placeholder expressions in fact have this exact behavior in "playgrounds mode" already. We could easily make it so that placeholders warn rather than error in normal source code as well.

-Joe

···

On Mar 29, 2017, at 8:11 AM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

On Mar 29, 2017, at 9:15 AM, Alex Blewitt <alblue@apple.com> wrote:

On 29 Mar 2017, at 14:10, Jonathan Hull <jhull@gbis.com> wrote:

I think the idea is that it also adds a warning so you can find it later.

@available(*, deprecated, message: "Don't forget to implement this")
func unimplemented<T>(_ file:String = #file,_ line:Int = #line) -> T {
  fatalError("Not implemented \(file):\(line)")
}

let f: String = unimplemented()

I was suggesting that it would be a useful addition to the language, not that it
necessarily needed new compiler support.

Does someone want to write a proposal for it?

Actually, looking more closely, we already have this behavior in playgrounds (and REPLs); it's only an error when you compile. Like, we literally do this:

      // Found it. Flag it as error (or warning, if in playground mode) for the
      // rest of the compiler pipeline and lex it as an identifier.
      if (LangOpts.Playground) {
        diagnose(TokStart, diag::lex_editor_placeholder_in_playground);
      } else {
        diagnose(TokStart, diag::lex_editor_placeholder);
      }

Could we change the compile-time error into a warning? Would that require an evolution proposal?

···

On Mar 29, 2017, at 4:14 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

On Mar 29, 2017, at 8:11 AM, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

I was suggesting that it would be a useful addition to the language, not that it
necessarily needed new compiler support.

Personally, what I'd like to see is for the existing <#whatever#> placeholder syntax to be treated as an unimplemented() call. That probably *would* require compiler support, although fortunately we already parse this syntax into an EditorPlaceholderExpr.

--
Brent Royal-Gordon
Architechies

Ah, you beat me to it. I would support reducing this to a warning.

-Joe

···

On Mar 29, 2017, at 4:21 PM, Brent Royal-Gordon via swift-evolution <swift-evolution@swift.org> wrote:

On Mar 29, 2017, at 4:14 PM, Brent Royal-Gordon <brent@architechies.com> wrote:

On Mar 29, 2017, at 8:11 AM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

I was suggesting that it would be a useful addition to the language, not that it
necessarily needed new compiler support.

Personally, what I'd like to see is for the existing <#whatever#> placeholder syntax to be treated as an unimplemented() call. That probably *would* require compiler support, although fortunately we already parse this syntax into an EditorPlaceholderExpr.

Actually, looking more closely, we already have this behavior in playgrounds (and REPLs); it's only an error when you compile. Like, we literally do this:

      // Found it. Flag it as error (or warning, if in playground mode) for the
      // rest of the compiler pipeline and lex it as an identifier.
      if (LangOpts.Playground) {
        diagnose(TokStart, diag::lex_editor_placeholder_in_playground);
      } else {
        diagnose(TokStart, diag::lex_editor_placeholder);
      }

Could we change the compile-time error into a warning? Would that require an evolution proposal?