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

fix #46918, unstable jl_binding_type behavior #46994

Merged
merged 1 commit into from
Oct 12, 2022
Merged

Conversation

JeffBezanson
Copy link
Member

fix #46918

Now it will return nothing before the binding has been resolved, and afterward fully look up the binding as declared by the owner.
I followed the comment here, which says the function doesn't resolve the binding.

@JeffBezanson JeffBezanson added bugfix This change fixes an existing bug modules labels Sep 30, 2022
@JeffBezanson
Copy link
Member Author

julia> ccall(:jl_binding_type, Any, (Any, Any), Main, :stderr)

julia> stderr
Base.TTY(RawFD(15) open, 0 bytes waiting)

julia> ccall(:jl_binding_type, Any, (Any, Any), Main, :stderr)
IO

julia> ccall(:jl_binding_type, Any, (Any, Any), Main, :stderr)
IO

@aviatesk
Copy link
Member

Maybe we want abstract_eval_globalref to try to resolve a binding always? Or will it cost too much?

@JeffBezanson
Copy link
Member Author

It shouldn't, to avoid inference having side effects.

@aviatesk
Copy link
Member

aviatesk commented Oct 1, 2022

Can we probably add this test case?

let # https://github.com/JuliaLang/julia/issues/46918
    code = quote
        res1 = ccall(:jl_binding_type, Any, (Any, Any), Main, :stderr)

        stderr

        res2 = ccall(:jl_binding_type, Any, (Any, Any), Main, :stderr)

        res3 = ccall(:jl_binding_type, Any, (Any, Any), Main, :stderr)

        print(stdout, res1, " ", res2, " ", res3)
    end |> string
    cmd = `$(Base.julia_cmd()) -e $code`
    stdout = IOBuffer()
    stderr = IOBuffer()
    @test success(pipeline(Cmd(cmd); stdout=stdout, stderr=stderr))
    @test String(take!(stdout)) == "nothing IO IO"
    @test isempty(String(take!(stderr)))
end

@JeffBezanson JeffBezanson added the needs tests Unit tests are required for this change label Oct 3, 2022
Now it will return `nothing` before the binding has been resolved,
and afterward fully look up the binding as declared by the owner.
@aviatesk aviatesk removed the needs tests Unit tests are required for this change label Oct 10, 2022
@aviatesk aviatesk merged commit 9104c20 into master Oct 12, 2022
@aviatesk aviatesk deleted the jb/bindingtyperes branch October 12, 2022 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jl_binding_type is very unstable
2 participants