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.

Sign up for GitHub

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

Jump to bottom

limit_type_size failure for Type{Type{..}} #40336

Closed
vtjnash opened this issue Apr 3, 2021 · 0 comments · Fixed by #40379
Closed

limit_type_size failure for Type{Type{..}} #40336

vtjnash opened this issue Apr 3, 2021 · 0 comments · Fixed by #40379
Labels
bug Indicates an unexpected problem or unintended behavior compiler:inference Type inference

Comments

@vtjnash
Copy link
Member

vtjnash commented Apr 3, 2021 

julia> f(@nospecialize(t)) = f(Type{t})
f (generic function with 1 method)

julia> @code_typed f(Int)
ERROR: StackOverflowError:

So clearly this type-limit isn't working:

        if isType(t) # allow taking typeof as Type{...}, but ensure it doesn't start nesting
            tt = unwrap_unionall(t.parameters[1])
            if isa(tt, DataType) && !isType(tt)
                is_derived_type_from_any(tt, sources, depth) && return t
            end
        end

Reported by @aviatesk
@vtjnash vtjnash added bug Indicates an unexpected problem or unintended behavior compiler:inference Type inference labels Apr 3, 2021
aviatesk added a commit to aviatesk/julia that referenced this issue Apr 4, 2021
@aviatesk
fix JuliaLang#40336, limit single-level nested Type signature
c6d23b0 
Previously `type_more_complex` returns `false` for `Type{Type{...}}`
compared against `Type{...}`. AFAIU this should return `true`, and
the change will fix JuliaLang#40336.
@aviatesk aviatesk mentioned this issue Apr 4, 2021
Closed
vtjnash added a commit that referenced this issue Apr 6, 2021
@vtjnash @aviatesk
inference: limit single-level nested Type signature
03b4aac 
Previously `type_more_complex` returns `false` for `Type{Type{...}}`
compared against `Type{...}`. Per comment there, this should return `true`,
but the code was ordered incorrect (it was also missing support for
Vararg, after the recent changes).

Fixes #40336

Co-authored-by: Shuhei Kadowaki <[email protected]>
@vtjnash vtjnash mentioned this issue Apr 6, 2021
Merged
vtjnash added a commit that referenced this issue Apr 9, 2021
@vtjnash @aviatesk
inference: limit single-level nested Type signature (#40379)
c2ec70c 
Previously `type_more_complex` returns `false` for `Type{Type{...}}`
compared against `Type{...}`. Per comment there, this should return `true`,
but the code was ordered incorrect (it was also missing support for
Vararg, after the recent changes).

Fixes #40336

Co-authored-by: Shuhei Kadowaki <[email protected]>
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this issue May 4, 2021
@vtjnash @aviatesk @ElOceanografo
inference: limit single-level nested Type signature (JuliaLang#40379)
8c6d188 
Previously `type_more_complex` returns `false` for `Type{Type{...}}`
compared against `Type{...}`. Per comment there, this should return `true`,
but the code was ordered incorrect (it was also missing support for
Vararg, after the recent changes).

Fixes JuliaLang#40336

Co-authored-by: Shuhei Kadowaki <[email protected]>
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this issue May 9, 2021
@vtjnash @aviatesk @antoine-levitt
inference: limit single-level nested Type signature (JuliaLang#40379)
eab1f65 
Previously `type_more_complex` returns `false` for `Type{Type{...}}`
compared against `Type{...}`. Per comment there, this should return `true`,
but the code was ordered incorrect (it was also missing support for
Vararg, after the recent changes).

Fixes JuliaLang#40336

Co-authored-by: Shuhei Kadowaki <[email protected]>
aviatesk added a commit that referenced this issue May 28, 2021
@aviatesk
fix #40814, fix type inference regression introduced in #40379
96e9e01 
Fixes the inference regression, while retaining the cases addressed by
<#40379>.

The same `Type`-name comparison pass (i.e. a branch of `isa(c, DataType) && t.name === c.name`)
can lead to more accurate result by recursive comparison than `isType`
check pass (i.e. a branch of `isType(t)`), and preferring the former
over the latter fixes the regression.

But just swapping the branch will lead to <#40336>,
and so this PR also implements additional check to make sure `type_more_complex`
still detects a single-level nesting correctly (especially, the `tt === c` parts).
@aviatesk aviatesk mentioned this issue May 28, 2021
Closed
aviatesk added a commit that referenced this issue May 31, 2021
@aviatesk
fix #40814, fix type inference regression introduced in #40379
c3490c0 
Fixes the inference regression, while retaining the cases addressed by
<#40379>.

The same `Type`-name comparison pass (i.e. a branch of `isa(c, DataType) && t.name === c.name`)
can lead to more accurate result by recursive comparison than `isType`
check pass (i.e. a branch of `isType(t)`), and preferring the former
over the latter fixes the regression.

But just swapping the branch will lead to <#40336>,
and so this PR also implements additional check to make sure `type_more_complex`
still detects a single-level nesting correctly (especially, the `tt === c` parts).
johanmon pushed a commit to johanmon/julia that referenced this issue Jul 5, 2021
@vtjnash @aviatesk @johanmon
inference: limit single-level nested Type signature (JuliaLang#40379)
62d7361 
Previously `type_more_complex` returns `false` for `Type{Type{...}}`
compared against `Type{...}`. Per comment there, this should return `true`,
but the code was ordered incorrect (it was also missing support for
Vararg, after the recent changes).

Fixes JuliaLang#40336

Co-authored-by: Shuhei Kadowaki <[email protected]>
@NHDaly NHDaly mentioned this issue Nov 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior compiler:inference Type inference
Projects
None yet
Milestone
No milestone
1 participant
@vtjnash