Disallowing unreachable code


(Peter Dillinger) #1

I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.

If you would like to see more defect examples from open-source software (other languages for the moment), I am able to dig up such things.

Thanks

···

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software


(Charles Srstka) #2

-1, for all the reasons others have already explained. This unnecessarily complicates the debugging process. If you ship code with warnings still in it, that’s your own fault.

Charles

···

On Mar 24, 2017, at 5:54 PM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.

If you would like to see more defect examples from open-source software (other languages for the moment), I am able to dig up such things.

Thanks

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(John McCall) #3

I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.

It was, indeed, something of a policy goal for awhile to not have any compiler warnings in Swift. The idea was that anything suspect should be an error. We deliberately backed away from that because it was actually really disruptive to the development experience: even very conscientious programmers who diligently fix all their warnings sometimes have code "in flight", so to speak, and want to be able to test and debug it without immediately making invasive edits. To use a germane example, if unreachable code were always an error, a programmer trying to debug a problem wouldn't be able to short-circuit a function by just adding a return; they'd also have to comment out the rest of the function.

I think we're comfortable with the current balance. It's understood that some programmers just ignore compiler warnings, but there are costs on the other side, too.

John.

···

On Mar 24, 2017, at 6:54 PM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

If you would like to see more defect examples from open-source software (other languages for the moment), I am able to dig up such things.

Thanks

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Charlie Monroe) #4

I'm personally -1 on this.

I find it useful from time to time when debugging certain features - instead of commenting some part of code (and potentially forgetting to uncomment it), I wrap it in an "if false" statement - while I get a warning about the code not being reachable (or the if statement always evaluating to false), I can test your code and then go by the warnings and remove the "if false" statements.

Similarly returning earlier. The warning is convenient as it warns you to review that part before releasing the code to the public.

If we take into account the argument that people commonly ignore compiler warnings, we should enable "treat warnings as errors" by default - which I'm sure is not the correct way to go.

···

On Mar 24, 2017, at 11:54 PM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.

If you would like to see more defect examples from open-source software (other languages for the moment), I am able to dig up such things.

Thanks

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Haravikk) #5

Which people? Personally warnings bug me no end; I'm not happy until they're all dealt with, so a warning is more than sufficient in my case.

While obviously writing unreachable code on purpose isn't the best way to do something, sometimes throwing some code into an if (false) or dropping in an early return to see if it fixes or triggers a problem is a perfectly reasonable way to do something in a quick and dirty way.

IMO a warning is more than sufficient for this, so that the programmer is reminded to go back and fix it.

I don't know if it's possible to elevate specific warnings to errors (I used to do this working with Java in Eclipse but have never wanted to in Xcode), if not then that may be worth considering as an alternative? But as a default I think a warning is sufficient, at least for me, as I'd rather be able to make code unreachable than have an error when it is.

···

On 24 Mar 2017, at 22:54, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.


(Joe Groff) #6

That's a fair concern, but unlike C compilers, where warnings vary widely among implementations and often aren't even on by default, Swift's warnings are considered as part of the language design and can't be turned off, so there's less opportunity or excuse to ignore them.

-Joe

···

On Mar 24, 2017, at 3:54 PM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.


(Joshua Alvarado) #7

This is more of a developer practice that a team implements than an actual
Swift/Xcode language feature. Also, I believe (not 100% sure) this wouldn't
work for functions/properties that are dynamically dispatched. The compiler
wouldn't know if the function will be called until runtime.

···

On Mon, Mar 27, 2017 at 11:15 AM, Charlie Monroe via swift-evolution < swift-evolution@swift.org> wrote:

I'm personally -1 on this.

I find it useful from time to time when debugging certain features -
instead of commenting some part of code (and potentially forgetting to
uncomment it), I wrap it in an "if false" statement - while I get a warning
about the code not being reachable (or the if statement always evaluating
to false), I can test your code and then go by the warnings and remove the
"if false" statements.

Similarly returning earlier. The warning is convenient as it warns you to
review that part before releasing the code to the public.

If we take into account the argument that people commonly ignore compiler
warnings, we should enable "treat warnings as errors" by default - which
I'm sure is not the correct way to go.

> On Mar 24, 2017, at 11:54 PM, Peter Dillinger via swift-evolution < > swift-evolution@swift.org> wrote:
>
> I don't see anything directly relevant to this in the archives, and I
haven't prepared a detailed proposal. But I'm raising the general idea
because I recently criticized Swift 3 for allowing unreachable code in a
blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/
swift-programming-language-design-part-2/ (search for "unreachable
code"). And I want you to have every opportunity to rectify this, even
though I'm in the business of finding defects the compiler doesn't. :slight_smile:
>
> Part of my argument is that people commonly ignore compiler warnings.
We see lots of defective code that would be (or is) caught by compiler
warnings but people don't pay attention.
>
> If you would like to see more defect examples from open-source software
(other languages for the moment), I am able to dig up such things.
>
> Thanks
>
> --
> Peter Dillinger, Ph.D.
> Software Engineering Manager, Coverity Analysis, Software Integrity
Group | Synopsys
> www.synopsys.com/software
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Joshua Alvarado
alvaradojoshua0@gmail.com


(Matthew Johnson) #8

I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.

It was, indeed, something of a policy goal for awhile to not have any compiler warnings in Swift. The idea was that anything suspect should be an error. We deliberately backed away from that because it was actually really disruptive to the development experience: even very conscientious programmers who diligently fix all their warnings sometimes have code "in flight", so to speak, and want to be able to test and debug it without immediately making invasive edits. To use a germane example, if unreachable code were always an error, a programmer trying to debug a problem wouldn't be able to short-circuit a function by just adding a return; they'd also have to comment out the rest of the function.

I think we're comfortable with the current balance. It's understood that some programmers just ignore compiler warnings, but there are costs on the other side, too.

+1. I like this being a warning for exactly the reasons you state. It’s very useful to sometimes be able to just insert an early return when debugging.

···

On Mar 27, 2017, at 12:25 PM, John McCall via swift-evolution <swift-evolution@swift.org> wrote:

On Mar 24, 2017, at 6:54 PM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

John.

If you would like to see more defect examples from open-source software (other languages for the moment), I am able to dig up such things.

Thanks

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution


(Peter Dillinger) #9

I should have clarified what I mean by "unreachable". I am referring to user code that is orphaned in a standard control flow graph construction, which does not perform any optimizations based on the values of expressions. The following has unreachable code:

  return 42;
  return -1; // unreachable

The following does not (from the blog post: "use 'if (false)' all you want"):

  if (false) {
    return -1;
  }

And the following, which allows you to bypass an existing function definition does not:

  if (true) {
    return -1;
  }

We call that "dead code" and in particular "intentional" because it's obvious from how it's written that the programmer intended to disable some code. The reason for disallowing unreachable code is that it's too easy get accidentally, which also makes the intent unclear.

···

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software


(Peter Dillinger) #10

To use a germane example, if unreachable code were always an error, a programmer trying to
debug a problem wouldn't be able to short-circuit a function by just adding a return;

True, but

they'd also have to comment out the rest of the function.

No, as you can do

  if (true) {
    return -1
  }

See clarifying reply on definition of "unreachable".

Returning to Charlie's reply:

Similarly returning earlier. The warning is convenient as it warns you to review that part
before releasing the code to the public.

Since swift implements a warning for the if(true) or if(false) cases, we still get that, and without the cost of accepting code with a serious chance of being accidentally wrong. I don't think anyone ever wrote if(true) or if(false) expecting some behavior other than what they wrote.

···

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software


(Peter Dillinger) #11

-1, for all the reasons others have already explained. This unnecessarily complicates the debugging process. If you ship code with warnings still in it, that’s your own fault.

All the obsolete/unfounded reasons? Please address my technical arguments directly, rather than in the (potentially stale/unfounded) abstract.

···

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software


(Chris Lattner) #12

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.

-Chris

···

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

On Mar 24, 2017, at 6:54 PM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:
I don't see anything directly relevant to this in the archives, and I haven't prepared a detailed proposal. But I'm raising the general idea because I recently criticized Swift 3 for allowing unreachable code in a blog post: https://blogs.synopsys.com/software-integrity/2017/03/24/swift-programming-language-design-part-2/ (search for "unreachable code"). And I want you to have every opportunity to rectify this, even though I'm in the business of finding defects the compiler doesn't. :slight_smile:

Part of my argument is that people commonly ignore compiler warnings. We see lots of defective code that would be (or is) caught by compiler warnings but people don't pay attention.

It was, indeed, something of a policy goal for awhile to not have any compiler warnings in Swift. The idea was that anything suspect should be an error. We deliberately backed away from that because it was actually really disruptive to the development experience: even very conscientious programmers who diligently fix all their warnings sometimes have code "in flight", so to speak, and want to be able to test and debug it without immediately making invasive edits. To use a germane example, if unreachable code were always an error, a programmer trying to debug a problem wouldn't be able to short-circuit a function by just adding a return; they'd also have to comment out the rest of the function.

I think we're comfortable with the current balance. It's understood that some programmers just ignore compiler warnings, but there are costs on the other side, too.


(David Sweeris) #13

-1, because having to write out “if(true) {…}” just because somebody might ignore a warning is annoying.

I wouldn’t object to the “—treatWarningsAsErrors” flag someone else mentioned.

- Dave Sweeris

···

On Mar 27, 2017, at 11:27 AM, Peter Dillinger via swift-evolution <swift-evolution@swift.org> wrote:

-1, for all the reasons others have already explained. This unnecessarily complicates the debugging process. If you ship code with warnings still in it, that’s your own fault.

All the obsolete/unfounded reasons? Please address my technical arguments directly, rather than in the (potentially stale/unfounded) abstract.


(John McCall) #14

To use a germane example, if unreachable code were always an error, a programmer trying to
debug a problem wouldn't be able to short-circuit a function by just adding a return;

True, but

they'd also have to comment out the rest of the function.

No, as you can do

if (true) {
   return -1
}

See clarifying reply on definition of "unreachable".

I'm not claiming that it's impossible to suppress the error. I'm saying that we made a conscious decision to not use hard errors for issues that merely *might* indicate a semantic mistake because doing so can interfere with the realities of development.

Note that such interference can be actively counter-productive. If a diagnostic is a warning, the programmer can safely put it out of mind because the warning will stick around until they fix it. If the diagnostic is instead an error, they'll have to immediately suppress it in order to get on with the build; that suppression mechanism will remain in place indefinitely until they come back and fix it.

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.

John.

···

On Mar 27, 2017, at 1:48 PM, Peter Dillinger <Peter.Dillinger@synopsys.com> wrote:

Returning to Charlie's reply:

Similarly returning earlier. The warning is convenient as it warns you to review that part
before releasing the code to the public.

Since swift implements a warning for the if(true) or if(false) cases, we still get that, and without the cost of accepting code with a serious chance of being accidentally wrong. I don't think anyone ever wrote if(true) or if(false) expecting some behavior other than what they wrote.

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software


(Jon Hull) #15

Yes, please.

···

On Mar 27, 2017, at 11:27 AM, John McCall via swift-evolution <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.


(Peter Dillinger) #16

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?

···

--
Peter Dillinger, Ph.D.
Software Engineering Manager, Coverity Analysis, Software Integrity Group | Synopsys
www.synopsys.com/software


(Chris Lattner) #17

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

-Chris

···

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?


(Peter Dillinger) #18

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.

-peter

···

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.


(Alex Blewitt) #19

You can do this already:

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

let f: String = unimplemented()

···

On 29 Mar 2017, at 11:38, Jonathan Hull via swift-evolution <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.


(David Sweeris) #20

Switches are required to be exhaustive in Swift, but code is not required to be called.

- Dave Sweeris

···

On Mar 28, 2017, at 21:55, Peter Dillinger via swift-evolution <swift-evolution@swift.org> 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?