toitlang / toit

Program your microcontrollers in a fast and robust high-level language.
https://toitlang.org/
GNU Lesser General Public License v2.1
1.23k stars 81 forks source link

"it" usage on nested blocks #2622

Closed kidandcat closed 3 weeks ago

kidandcat commented 3 weeks ago

When you refer to it inside a nested block, if you intended to refer the outer it, toit will error because it thinks your inner block expects an argument.

main: 
  [1, 2].do:
    print it
    catch --trace:
      throw it

Trace:

1
2
Called block with too few arguments.
Got: 0 Expected: 1.
Target:
     main.<block>.<block>      bug.toit:5:18

  0: catch.<block>             <sdk>/core/exceptions.toit:124:10
  1: catch                     <sdk>/core/exceptions.toit:122:1
  2: catch                     <sdk>/core/exceptions.toit:73:10
  3: main.<block>              bug.toit:5:5
  4: SmallArray_.do_           <sdk>/core/collections.toit:954:3
  5: List_.do                  <sdk>/core/collections.toit:1933:19
  6: main                      bug.toit:3:10

I'm learning Toit, and this issue got me stuck for a while, at least I would add a warning to the exception trace indicating the user if he is falling into this.

Real use case were this happened: I was doing an http request per item in a List

...
service.environments.do:
    err := catch --trace --unwind:
          request := client.get it.host it.path
          elem.color = request.status-code < 300 ? 0x00FF00 : 0xFF0000
    if err:
          print "Exception catched: $err"
          elem.color = 0xFF0000
...
kidandcat commented 3 weeks ago

BTW, I solved it just by naming the do block param, but it took me a while to notice what was the error.

service.environments.do: |env|
floitsch commented 3 weeks ago

It's, unfortunately, a common mistake.

  • Is this the intended behaviour?

It is, although it's not really helpful here. The problem is, that the error could be on both sides: the caller provided a block that wants an argument that it won't get, or the callee calls the block but forgets to provide an argument.

In the compiler we would probably print both locations and give the user the information that either one is probably wrong. Since the error here is dynamic, we don't have that option (as easily).

  • Shouldn't toit be aware that catch is not calling the block with any arguments and extrapolate that the it refers to the outer loop?
    • maybe typing it somehow?

That would be too magic, imho. When looking at a source, you shouldn't need to know the type of targets to know which block an it resolves too. It should always be the inner-most one.

That said, there are two things we plan to do to make this better:

Eventually, it would be nice to color the it in some way that makes it clear where it comes from, but that's not something I see happening in the near future.

Side note: I recommend to use an indentation of 2 (and not 4). The syntax highlighter works better with that indentation.

I'm closing this issue. Not, because I don't agree, but because the two actionable items (better exception handling, and typing) are tracked elsewhere. We do appreciate this kind of feedback, so please keep them coming.