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 lowering of closure supertypes #44974

Merged
merged 2 commits into from
Apr 14, 2022
Merged

Conversation

JeffBezanson
Copy link
Sponsor Member

The supertype should be set earlier, to avoid assigning a global to a type object that's not fully initialized.

The supertype should be set earlier, to avoid assigning a global
to a type object that's not fully initialized.
@JeffBezanson JeffBezanson added the compiler:lowering Syntax lowering (compiler front end, 2nd stage) label Apr 13, 2022
@vtjnash vtjnash added the status:merge me PR is reviewed. Merge when all tests are passing label Apr 14, 2022
@StefanKarpinski
Copy link
Sponsor Member

Is there some symptom this fixes? Any test?

@JeffBezanson
Copy link
Sponsor Member Author

My understanding is that this is needed by JuliaInterpreter, since it operates on the type object while it's in an invalid state, and this reduces the window where that can happen. I don't think it's possible to see any change from this just using the normal system.

@vtjnash vtjnash merged commit 57b74ba into master Apr 14, 2022
@vtjnash vtjnash deleted the jb/closuresupertypelowering branch April 14, 2022 20:31
@DilumAluthge DilumAluthge removed the status:merge me PR is reviewed. Merge when all tests are passing label Apr 14, 2022
aviatesk added a commit to JuliaDebug/LoweredCodeUtils.jl that referenced this pull request Apr 19, 2022
Lowering for a closure construct has been changed by JuliaLang/julia#44974 as like:
```julia
julia> using LoweredCodeUtils, JuliaInterpreter

julia> ex = quote
               function fouter(x)
                   finner(::Float16) = 2x
                   return finner(Float16(1))
               end
           end
quote
    #= REPL[19]:2 =#
    function fouter(x)
        #= REPL[19]:2 =#
        #= REPL[19]:3 =#
        finner(::Float16) = begin
                #= REPL[19]:3 =#
                2x
            end
        #= REPL[19]:4 =#
        return finner(Float16(1))
    end
end

julia> Core.eval(@__MODULE__, ex)
fouter (generic function with 1 method)

julia> Frame(@__MODULE__, ex).framecode.src
```

```diff
diff --git a/before.jl b/after.jl
index 24b26cb..728d385 100644
--- a/before.jl
+++ b/after.jl
@@ -12,9 +12,9 @@ CodeInfo(
 │        const var"#finner#2"
 │        Core.TypeVar(:x, Core.Any)
 │   %4 = Core._structtype(Main, Symbol("#finner#2"), Core.svec(%3), Core.svec(:x), Core.svec(), false, 1)
+│        Core._setsuper!(%4, Core.Function)
 │        var"#finner#2" = %4
-│        Core._setsuper!(var"#finner#2", Core.Function)
-│        Core._typebody!(var"#finner#2", Core.svec(%3))
+│        Core._typebody!(%4, Core.svec(%3))
 └──      return nothing
 )))
 │       ($(QuoteNode(Core.svec)))(var"#finner#2", Float16)
```
aviatesk added a commit to JuliaDebug/LoweredCodeUtils.jl that referenced this pull request Apr 19, 2022
Lowering for a closure construct has been changed by JuliaLang/julia#44974 as like:
```julia
julia> using LoweredCodeUtils, JuliaInterpreter

julia> ex = quote
               function fouter(x)
                   finner(::Float16) = 2x
                   return finner(Float16(1))
               end
           end
quote
    #= REPL[19]:2 =#
    function fouter(x)
        #= REPL[19]:2 =#
        #= REPL[19]:3 =#
        finner(::Float16) = begin
                #= REPL[19]:3 =#
                2x
            end
        #= REPL[19]:4 =#
        return finner(Float16(1))
    end
end

julia> Core.eval(@__MODULE__, ex)
fouter (generic function with 1 method)

julia> Frame(@__MODULE__, ex).framecode.src
```

```diff
diff --git a/before.jl b/after.jl
index 24b26cb..728d385 100644
--- a/before.jl
+++ b/after.jl
@@ -12,9 +12,9 @@ CodeInfo(
 │        const var"#finner#2"
 │        Core.TypeVar(:x, Core.Any)
 │   %4 = Core._structtype(Main, Symbol("#finner#2"), Core.svec(%3), Core.svec(:x), Core.svec(), false, 1)
+│        Core._setsuper!(%4, Core.Function)
 │        var"#finner#2" = %4
-│        Core._setsuper!(var"#finner#2", Core.Function)
-│        Core._typebody!(var"#finner#2", Core.svec(%3))
+│        Core._typebody!(%4, Core.svec(%3))
 └──      return nothing
 )))
 │       ($(QuoteNode(Core.svec)))(var"#finner#2", Float16)
```
aviatesk added a commit to JuliaDebug/LoweredCodeUtils.jl that referenced this pull request Apr 19, 2022
Lowering for a closure construct has been changed by JuliaLang/julia#44974 as like:
```julia
julia> using LoweredCodeUtils, JuliaInterpreter

julia> ex = quote
               function fouter(x)
                   finner(::Float16) = 2x
                   return finner(Float16(1))
               end
           end
quote
    #= REPL[19]:2 =#
    function fouter(x)
        #= REPL[19]:2 =#
        #= REPL[19]:3 =#
        finner(::Float16) = begin
                #= REPL[19]:3 =#
                2x
            end
        #= REPL[19]:4 =#
        return finner(Float16(1))
    end
end

julia> Core.eval(@__MODULE__, ex)
fouter (generic function with 1 method)

julia> Frame(@__MODULE__, ex).framecode.src
```

```diff
diff --git a/before.jl b/after.jl
index 24b26cb..728d385 100644
--- a/before.jl
+++ b/after.jl
@@ -12,9 +12,9 @@ CodeInfo(
 │        const var"#finner#2"
 │        Core.TypeVar(:x, Core.Any)
 │   %4 = Core._structtype(Main, Symbol("#finner#2"), Core.svec(%3), Core.svec(:x), Core.svec(), false, 1)
+│        Core._setsuper!(%4, Core.Function)
 │        var"#finner#2" = %4
-│        Core._setsuper!(var"#finner#2", Core.Function)
-│        Core._typebody!(var"#finner#2", Core.svec(%3))
+│        Core._typebody!(%4, Core.svec(%3))
 └──      return nothing
 )))
 │       ($(QuoteNode(Core.svec)))(var"#finner#2", Float16)
```
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants