modularml / mojo

The Mojo Programming Language
https://docs.modular.com/mojo/manual/
Other
22.89k stars 2.58k forks source link

[BUG] try..finally/except and raises #3410

Open f-saez opened 3 weeks ago

f-saez commented 3 weeks ago

Bug description

Hi,

I've noticed a strange behavior with the last nightly.

from testing import assert_true

fn is_title(t : String) -> String:
    var result = String()
    if t.startswith("TITLE"):
        try:
            var t1 = t.split(" ")
            if len(t1)>1:
                result = t1[1]
        finally:
            pass
    return result

def main():
    assert_true(is_title("TITLE something"))

it doesn't compile, with the following error message :

note: try surrounding the call in a 'try' block
            var t1 = t.split("\"")

If I change finally by except

fn is_title(t : String) -> String:
    var result = String()
    if t.startswith("TITLE"):
        try:
            var t1 = t.split(" ")
            if len(t1)>1:
                result = t1[1]
        except:
            pass
    return result

everything's fine again.

Steps to reproduce

run the small piece of code

System information

- OS : Fedora 40 x86_64
- mojo -v => mojo 2024.8.2201 (f6a255dc)
- modular -v => modular 0.9.2 (b3079bd5)
ematejska commented 3 weeks ago

Confirming that this is a change in behavior since 24.4. To me this seems like a correct change to require that there is an except statement or that the function raises here but the error message doesn't look quite right. @Mogball for his view on this as well and what this needs.

Mogball commented 3 weeks ago

This is correct behaviour. We should improve the error message here, since it can be confusing. The try block needs an except region since the surrounding context is not raising.

Moosems commented 3 weeks ago

@Mogball What about requiring raises if there is no except clause? As of python 3.12 the following raises no syntax errors (even when you replace the raise with a pass) but has simply has no error handling:

try:
    raise Exception()
finally:
    pass
Mogball commented 3 weeks ago

@Mogball What about requiring raises if there is no except clause? As of python 3.12 the following raises no syntax errors (even when you replace the raise with a pass) but has simply has no error handling:

try:
    raise Exception()
finally:
    pass

In Python, this is equivalent to writing

try:
    raise Exception()
except:
    raise
finally:
    pass

I.e., the errors are passthrough the try-finally clause. In Mojo, the behaviour is the same.

def try_finally():
    try:
        raise "whoops"
    finally:
        pass

The problem in the example reported in the issue is that the try-finally clause itself is not in a context that can raise, thus it cannot implicitly re-raise. We should improve the error message here to be more specific!

Moosems commented 3 weeks ago

Makes sense, thanks for clarifying :)