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

false positive error from a type constructor with parameterized vector field #154

Closed
Octogonapus opened this issue Apr 9, 2021 · 2 comments · Fixed by #252
Closed

false positive error from a type constructor with parameterized vector field #154

Octogonapus opened this issue Apr 9, 2021 · 2 comments · Fixed by #252
Labels
abstract interpretation Improvements to abstract interpretation

Comments

@Octogonapus
Copy link

Octogonapus commented Apr 9, 2021

I am not sure how to specify the concretization pattern for this code:

const MyType = AbstractVector{<:AbstractString}

struct Foo
    x::MyType
end

I tried using: report_file("main.jl"; analyze_from_definitions = true, concretization_patterns = [:(MyType = x_)])
but that results in this error:

┌ @ main.jl:4 Base.convert(Core.fieldtype(Foo, 1), x)
│┌ @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/factorization.jl:58 _(f)
││ no matching method found for call signature: _::Type{AbstractVector{var"#s20"} where var"#s20"<:AbstractString}(f::LinearAlgebra.Factorization)
│└───────────────────────────────────────────────────────────────────────────────────────────────────────────────

I am not sure how LinearAlgebra has entered the picture. Any advice on what the concretization pattern should be here? I am using JET v0.1.0 and Julia 1.6.0.

@aviatesk
Copy link
Owner

aviatesk commented Apr 9, 2021

Hi, actually this error is because of the abstract interpretation, not a missing concretization.
(rather, type aliases are always concretized, and so don't need to specify concretization_patterns for this case)

The analysis report comes from the analyze_from_definitions configuration.
struct definition internally defines its constructor like below:

julia> JET.@lwr struct Foo
           x::MyType
       end
:($(Expr(:thunk, CodeInfo(
     @ none within `top-level scope'
1 ──       global Foo
│          const Foo
│    %3  = Core.svec()
│    %4  = Core.svec(:x)
│          #217#Foo = Core._structtype(Main, :Foo, %3, %4, false, 1)
│          Core._setsuper!(#217#Foo, Core.Any)
│    %7  = $(Expr(:isdefined, :Foo))
└───       goto #6 if not %7
2 ── %9  = Foo
│    %10 = Core._equiv_typedef(%9, #217#Foo)
└───       goto #4 if not %10
3 ──       #217#Foo = %9
└───       goto #5
4 ──       Foo = #217#Foo
5 ┄─       goto #7
6 ──       Foo = #217#Foo
7 ┄─ %17 = #217#Foo
│    %18 = Core.svec(MyType)
│          Core._typebody!(%17, %18)
│          global Foo
│    %21 = Core.Any === MyType
└───       goto #9 if not %21
8 ──       goto #10
9 ──       $(Expr(:method, :Foo))
│    %25 = Core.Typeof(Foo)
│    %26 = Core.svec(%25, MyType)
│    %27 = Core.svec()
│    %28 = Core.svec(%26, %27, $(QuoteNode(:(#= none:2 =#))))
└───       $(Expr(:method, :Foo, :(%28), CodeInfo(
    @ none:2 within `none'
1 ─ %1 = %new(Foo, x)
└──      return %1
)))
10 ┄       $(Expr(:method, :Foo))
│    %31 = Core.Typeof(Foo)
│    %32 = Core.svec(%31, Core.Any)
│    %33 = Core.svec()
│    %34 = Core.svec(%32, %33, $(QuoteNode(:(#= none:2 =#))))
$(Expr(:method, :Foo, :(%34), CodeInfo(
    @ none:2 within `none'
1 ─ %1 = Foo
│   %2 = Core.fieldtype(%1, 1)
│   %3 = Base.convert(%2, x)
│   %4 = %new(%1, %3)
└──      return %4
)))
└───       return nothing
))))

and the error is reported at Base.convert(%2::AbstractVector{<:AbstractString}, x::Any).

That said, this false positive frequently happens when using analyze_from_definitions=true, and so I think JET needs to fix this somehow.

@aviatesk aviatesk changed the title Concretization of global type alias false positive error from a type constructor with parameterized vector field Apr 9, 2021
@aviatesk
Copy link
Owner

aviatesk commented May 6, 2021

Easy workaround for now is to set max_methods=1 configuration: report_file("main.jl"; analyze_from_definitions = true, concretization_patterns = [:(MyType = x_)], max_methods=1)

I'm considering making this default though.

@aviatesk aviatesk added the abstract interpretation Improvements to abstract interpretation label May 8, 2021
aviatesk added a commit that referenced this issue Sep 13, 2021
For `BasicPass`, we skip errors reported from abstract frames.

- fixes #184
- fixes #154
aviatesk added a commit that referenced this issue Sep 13, 2021
For `BasicPass`, we skip errors reported from abstract frames.

- fixes #184
- fixes #154
aviatesk added a commit that referenced this issue Sep 13, 2021
For `BasicPass`, we skip errors reported from abstract frames.

- fixes #184
- fixes #154
aviatesk added a commit that referenced this issue Sep 13, 2021
For `BasicPass`, we skip errors reported from abstract frames.

- fixes #184
- fixes #154
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abstract interpretation Improvements to abstract interpretation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants