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

inference: Fix handling of :throw_undef_if_not #53875

Merged
merged 1 commit into from
Mar 27, 2024
Merged

inference: Fix handling of :throw_undef_if_not #53875

merged 1 commit into from
Mar 27, 2024

Conversation

Keno
Copy link
Member

@Keno Keno commented Mar 27, 2024

This stmt type doesn't usually get introduced until IR conversion, but we do allow it in inference (to allow it to be emitted by packages like Diffractor). However, we didn't have a test for it, so the code path grew a bug. Fix that and also allow this as a frontend form, so it can be tested without resorting to generated functions.

This stmt type doesn't usually get introduced until IR conversion, but we do allow it in
inference (to allow it to be emitted by packages like Diffractor). However, we didn't
have a test for it, so the code path grew a bug. Fix that and also allow this as a
frontend form, so it can be tested without resorting to generated functions.
@@ -2625,7 +2625,7 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp
effects = EFFECTS_UNKNOWN
end
elseif ehead === :throw_undef_if_not
condt = argextype(stmt.args[2], ir)
condt = abstract_eval_value(interp, e.args[2], vtypes, sv)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(we should occasionally run something like JET on Julia base)

@Keno Keno merged commit 0e2409a into master Mar 27, 2024
7 checks passed
@Keno Keno deleted the kf/tuin branch March 27, 2024 14:29
aviatesk pushed a commit that referenced this pull request Apr 4, 2024
#53875 allowed
`:throw_undef_if_not` as a frontend form.

However, the `UndefVarError` being tested is thrown because the first
argument is resolved to a global ref:

```julia
julia> @eval function has_tuin()
           $(Expr(:throw_undef_if_not, :x, false))
       end
has_tuin (generic function with 1 method)

julia> @code_lowered has_tuin() # master
CodeInfo(
1 ─ %1 = $(Expr(:throw_undef_if_not, :(Main.x), false))
└──      return %1
)

julia> @code_lowered has_tuin() # this pr
CodeInfo(
1 ─ %1 = $(Expr(:throw_undef_if_not, :x, false))
└──      return %1
)
```

This change skips this global ref resolution for the first argument and
fixes a typo which would throw an error in case of non-const second
argument.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants