Warn for unreachable code after todo and panic

[manveru]

Right now any argument given is simply ignored, leading to a bit confusing behavior.
While it may be syntactically possible to give an argument to the two keywords, they will mostly be ignored right now.

The tour teaches that the correct way to give a message is using the as keyword, but that is rather late in the tour and easily forgotten later, so it would be nice to give a hint and remind people of that gotcha.

[inoas]

the next line could be a valid string expression and at least todo could be used to stop the code execution at that point? the compiler could warn that followup expressions in the same block are not reachable after todo/panic if that's not the case?

[lpil]

Thank you

[giacomocavalieri]

If I can give my two cents here: the Rust compiler does that and I really dislike this behaviour. This is just my experience but here's what happens to me most of the times: I'm quickly writing some code and put a todo somewhere just to make the code compile, and now I get two warnings:

pub fn main() {
  let a = todo
//        ^^^^ Todo used: your program will crash
  wibble(1, a)
//^^^^^^^^^^^^ Unreachable
  "some stuff"
}

What I just wanted was for the compiler to tell me where a todo is, not that what comes below is unreachable, I already know that since it tells me the program will crash here.

But now I get double the warnings and the second one is moving my focus away from the source of the problem (the todo) and is asking me to focus on the result of the problem (the following lines are unreachable because of the todo). That's not useful at all: the solution is still taking care of that todo as the first warning was already saying.
At this point my reaction is "well this is not that useful" and I start dismissing that second warning, which is not something I want people to do with warnings.

This is just my experience but I hope you get the idea of the problem here: I'd rather have a focused warning for this specific confusing case rather than a warning that is just redundant/distracting 80% of the times

---

What could this look like? Something like this:

warning: Todo used as a function
  ┌─ /src/main.gleam:2:3
  │
2 │   todo(expr)
  │       ^^^^^^ This won't appear in the error message

`todo` is not a function and will crash before it can do
anything with these arguments.

Hint: if you want to display an error message you should
write `todo as "your error message here"`

See: https://tour.gleam.run/advanced-features/todo/

So now we have a second warning just for this possible confusing case -"Where's my error message??"- that explains clearly how to solve your issue. A generic "This code is unused" could be just confusing and redundant most of the times

[lpil]

What I just wanted was for the compiler to tell me where a todo is, not that what comes below is unreachable, I already know that since it tells me the program will crash here.

The problem here is that while you in this case do know, we have had folks in the past be confused and not realise that some code was unreachable. Having said that, it was due to code that does not return rather than todo or panic.

How about we have that specialised error you've shared there, and we have an unreachable warning for panic specifically?

[giacomocavalieri]

I think that would be great! I can work on this one

[lpil]

Thank you

[giacomocavalieri]

I have a couple of question regarding warnings, how do we want to display them? For example:

• pub fn main() {
    panic
    let n = 1
    n + 1
  }

  Do we highlight just the line after panic or the whole unreachable group of statements?

  warning: Unreachable code
    ┌─ /src/warning/wrn.gleam:4:3
    │  
  4 │ ╭   let n = 1
  5 │ │   { n + 2 } * 3
    │ ╰───────────────^
  
  This code is unreachable since it comes after a `panic`.

• What if panic is inside a list/function call/tuple, what do we highlight here?

  pub fn main() {
    [1, 2, panic, 3, 4]
  }

  What if panic is the last item of the list, does that change anything in the way we want to report the error?

  pub fn main() {
    [1, 2, panic]
  }

  What if the list is not the last expression in a block?

  pub fn main() {
    let l = [1, 2, panic, 3, 4]
    [0, ..l]
  }
  

• Another case where I'm not sure what would be best is if there's multiple blocks with a panic each:

  pub fn main() {
    let a = {
      panic
      1 + 1
    }
    panic 
    a + 1
  }

  Do we highlight everything below the first panic? Or do we raise two warnings, one highlighting 1 + 1 and one highlighting a + 1? What if the first panic is the last expression of its block (that would still make everything unreachable below it)?

[lpil]

I think copying whatever Rust does would be good.