Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is try-catch scoping as hard as let and for? #34668

Open
tkf opened this issue Feb 5, 2020 · 6 comments
Open

Is try-catch scoping as hard as let and for? #34668

tkf opened this issue Feb 5, 2020 · 6 comments
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) kind:minor change Marginal behavior change acceptable for a minor release

Comments

@tkf
Copy link
Member

tkf commented Feb 5, 2020

Given that let x and for x in ... introduce a hard-scope for x, I find that it's inconsistent that the scoping of try ... catch err; ... end is ambiguous.

But I couldn't follow all the comments in #33864 and related issues so I may be missing something. I only found that it was explicitly decided that let uses a hard-scope #33864 (comment).

MWE:

julia> VERSION
v"1.5.0-DEV.230"

julia> include_string(Main, """
       x = let x
           x = 1
       end

       for x in 1:1
           x = 1
       end
       """)  # no warning

julia> include_string(Main, """
       err = try
           error("hello")
       catch err
           err
       end
       """)
┌ Warning: Assignment to `err` in soft scope is ambiguous because a global variable by the same name exists: `err` will be treated as a new local. Disambiguate by using `local err` to suppress this warning or `global err` to assign to the existing global variable.
└ @ string:1
ErrorException("hello")
@StefanKarpinski
Copy link
Sponsor Member

for does not introduce a hard scope, only let and functions do. The loop variable in a for loop is always local, however.

@tkf
Copy link
Member Author

tkf commented Feb 5, 2020

That's what I meant to say. Shouldn't the exception variable (err in try ... catch err; ... end) be always local?

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Feb 5, 2020

I suppose that does seem quite similar to the loop variable in a for loop. cc @JeffBezanson

@JeffBezanson
Copy link
Sponsor Member

Yes, I agree the catch block var should be let-bound. I was surprised to see (this really seems like a mistake of some sort) that we just lower this to a plain assignment. So for example

julia> function f()
         local x
         try
           error(2)
         catch x
         end
         x
       end
f (generic function with 1 method)

julia> f()
ErrorException("2")

That can't have been what we intended, but technically this is a breaking change.

@tkf
Copy link
Member Author

tkf commented Feb 6, 2020

Actually, I sometimes write code that relies on it. Something like

@testcase begin
    err = nothing
    @test try
        f()
        false
    catch err
        true
    end
    @test occursin("some error message", sprint(showerror, err))
end

I thought it was weird but I didn't think carefully back then when I started using it. (Before this I was using err = try ... catch err; err; end pattern.)

I personally prefer "tightening" the scope (so breaking the above code) but I'd imagine someone else might be using it already due to Hyrum's law. But if it is only me by some kind of miracle, please go ahead and change it :)

@StefanKarpinski
Copy link
Sponsor Member

Definitely worth a PkgEval run to see if it breaks much. Hopefully not!

@vtjnash vtjnash added compiler:lowering Syntax lowering (compiler front end, 2nd stage) kind:minor change Marginal behavior change acceptable for a minor release labels Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) kind:minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Development

No branches or pull requests

4 participants