Skip to content

Commit

Permalink
make UndefVarError messages more precise and informative (#51979)
Browse files Browse the repository at this point in the history
Record the 'scope' of the variable that was undefined (the Module, or a
descriptive word such as :local or :static_parameter). Add that scope to
the error message, and expand the hint suggestions added by the REPL to
include more specific advice on common mistakes:

  - forgetting to set an initial value
  - forgetting to import a global
  - creating a local of the same name as a global
  - not matching a static parameter in a signature subtype

Fixes #17062 (although more could probably be done to search for typos
using REPL.string_distance and getting the method from stacktrace)
Fixes #18877
Fixes #25263
Fixes #35126
Fixes #39280
Fixes #41728
Fixes #48731
Fixes #49917
Fixes #50369
  • Loading branch information
vtjnash committed Nov 8, 2023
1 parent 560ede5 commit 449c7a2
Show file tree
Hide file tree
Showing 32 changed files with 186 additions and 100 deletions.
3 changes: 3 additions & 0 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ struct StackOverflowError <: Exception end
struct UndefRefError <: Exception end
struct UndefVarError <: Exception
var::Symbol
scope # a Module or Symbol or other object describing the context where this variable was looked for (e.g. Main or :local or :static_parameter)
UndefVarError(var::Symbol) = new(var)
UndefVarError(var::Symbol, @nospecialize scope) = new(var, scope)
end
struct ConcurrencyViolationError <: Exception
msg::AbstractString
Expand Down
7 changes: 4 additions & 3 deletions base/docs/basedocs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1818,14 +1818,14 @@ In these examples, `a` is a [`Rational`](@ref), which has two fields.
nfields

"""
UndefVarError(var::Symbol)
UndefVarError(var::Symbol, [scope])
A symbol in the current scope is not defined.
# Examples
```jldoctest
julia> a
ERROR: UndefVarError: `a` not defined
ERROR: UndefVarError: `a` not defined in `Main`
julia> a = 1;
Expand Down Expand Up @@ -2346,7 +2346,8 @@ See also [`setproperty!`](@ref Base.setproperty!) and [`getglobal`](@ref)
julia> module M end;
julia> M.a # same as `getglobal(M, :a)`
ERROR: UndefVarError: `a` not defined
ERROR: UndefVarError: `a` not defined in `M`
Suggestion: check for spelling errors or missing imports. No global of this name exists in this module.
julia> setglobal!(M, :a, 1)
1
Expand Down
10 changes: 10 additions & 0 deletions base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,16 @@ showerror(io::IO, ex::UndefKeywordError) =

function showerror(io::IO, ex::UndefVarError)
print(io, "UndefVarError: `$(ex.var)` not defined")
if isdefined(ex, :scope)
scope = ex.scope
if scope isa Module
print(io, " in `$scope`")
elseif scope === :static_parameter
print(io, " in static parameter matching")
else
print(io, " in $scope scope")
end
end
Experimental.show_error_hints(io, ex)
end

Expand Down
4 changes: 2 additions & 2 deletions base/logging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -335,12 +335,12 @@ function logmsg_code(_module, file, line, level, message, exs...)
checkerrors = nothing
for kwarg in reverse(log_data.kwargs)
if isa(kwarg.args[2].args[1], Symbol)
checkerrors = Expr(:if, Expr(:isdefined, kwarg.args[2]), checkerrors, Expr(:call, Expr(:core, :UndefVarError), QuoteNode(kwarg.args[2].args[1])))
checkerrors = Expr(:if, Expr(:isdefined, kwarg.args[2]), checkerrors, Expr(:call, Expr(:core, :UndefVarError), QuoteNode(kwarg.args[2].args[1]), QuoteNode(:local)))
end
end
if isa(message, Symbol)
message = esc(message)
checkerrors = Expr(:if, Expr(:isdefined, message), checkerrors, Expr(:call, Expr(:core, :UndefVarError), QuoteNode(message.args[1])))
checkerrors = Expr(:if, Expr(:isdefined, message), checkerrors, Expr(:call, Expr(:core, :UndefVarError), QuoteNode(message.args[1]), QuoteNode(:local)))
end
logrecord = quote
let err = $checkerrors
Expand Down
7 changes: 4 additions & 3 deletions doc/src/manual/control-flow.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ julia> test(1,2)
x is less than y.
julia> test(2,1)
ERROR: UndefVarError: `relation` not defined
ERROR: UndefVarError: `relation` not defined in local scope
Stacktrace:
[1] test(::Int64, ::Int64) at ./none:7
```
Expand Down Expand Up @@ -458,7 +458,7 @@ julia> for j = 1:3
3
julia> j
ERROR: UndefVarError: `j` not defined
ERROR: UndefVarError: `j` not defined in `Main`
```

```jldoctest
Expand Down Expand Up @@ -862,7 +862,8 @@ end
else
foo
end
ERROR: UndefVarError: `foo` not defined
ERROR: UndefVarError: `foo` not defined in `Main`
Suggestion: check for spelling errors or missing imports. No global of this name exists in this module.
```
Use the [`local` keyword](@ref local-scope) outside the `try` block to make the variable
accessible from anywhere within the outer scope.
Expand Down
4 changes: 2 additions & 2 deletions doc/src/manual/distributed-computing.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ julia> rand2(2,2)
1.15119 0.918912
julia> fetch(@spawnat :any rand2(2,2))
ERROR: RemoteException(2, CapturedException(UndefVarError(Symbol("#rand2"))
ERROR: RemoteException(2, CapturedException(UndefVarError(Symbol("#rand2"))))
Stacktrace:
[...]
```
Expand Down Expand Up @@ -209,7 +209,7 @@ MyType(7)
julia> fetch(@spawnat 2 MyType(7))
ERROR: On worker 2:
UndefVarError: `MyType` not defined
UndefVarError: `MyType` not defined in `Main`
julia> fetch(@spawnat 2 DummyModule.MyType(7))
Expand Down
6 changes: 3 additions & 3 deletions doc/src/manual/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ julia> module Foo
julia> Foo.foo()
ERROR: On worker 2:
UndefVarError: `Foo` not defined
UndefVarError: `Foo` not defined in `Main`
Stacktrace:
[...]
```
Expand All @@ -746,7 +746,7 @@ julia> @everywhere module Foo
julia> Foo.foo()
ERROR: On worker 2:
UndefVarError: `gvar` not defined
UndefVarError: `gvar` not defined in `Main.Foo`
Stacktrace:
[...]
```
Expand Down Expand Up @@ -782,7 +782,7 @@ bar (generic function with 1 method)
julia> remotecall_fetch(bar, 2)
ERROR: On worker 2:
UndefVarError: `#bar` not defined
UndefVarError: `#bar` not defined in `Main`
[...]
julia> anon_bar = ()->1
Expand Down
4 changes: 2 additions & 2 deletions doc/src/manual/metaprogramming.md
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ julia> ex = :(a + b)
:(a + b)
julia> eval(ex)
ERROR: UndefVarError: `b` not defined
ERROR: UndefVarError: `b` not defined in `Main`
[...]
julia> a = 1; b = 2;
Expand All @@ -397,7 +397,7 @@ julia> ex = :(x = 1)
:(x = 1)
julia> x
ERROR: UndefVarError: `x` not defined
ERROR: UndefVarError: `x` not defined in `Main`
julia> eval(ex)
1
Expand Down
6 changes: 3 additions & 3 deletions doc/src/manual/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ julia> using .A, .B
julia> f
WARNING: both B and A export "f"; uses of it in module Main must be qualified
ERROR: UndefVarError: `f` not defined
ERROR: UndefVarError: `f` not defined in `Main`
```

Here, Julia cannot decide which `f` you are referring to, so you have to make a choice. The following solutions are commonly used:
Expand Down Expand Up @@ -404,7 +404,7 @@ x = 0

module Sub
using ..TestPackage
z = y # ERROR: UndefVarError: `y` not defined
z = y # ERROR: UndefVarError: `y` not defined in `Main`
end

y = 1
Expand All @@ -420,7 +420,7 @@ For similar reasons, you cannot use a cyclic ordering:
module A

module B
using ..C # ERROR: UndefVarError: `C` not defined
using ..C # ERROR: UndefVarError: `C` not defined in `Main.A`
end

module C
Expand Down
13 changes: 7 additions & 6 deletions doc/src/manual/variables-and-scoping.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ julia> module B
julia> module D
b = a # errors as D's global scope is separate from A's
end;
ERROR: UndefVarError: `a` not defined
ERROR: UndefVarError: `a` not defined in `D`
Suggestion: check for spelling errors or missing imports. No global of this name exists in this module.
```

If a top-level expression contains a variable declaration with keyword `local`,
Expand Down Expand Up @@ -187,7 +188,7 @@ julia> greet()
hello
julia> x # global
ERROR: UndefVarError: `x` not defined
ERROR: UndefVarError: `x` not defined in `Main`
```

Inside of the `greet` function, the assignment `x = "hello"` causes `x` to be a new local variable
Expand Down Expand Up @@ -256,7 +257,7 @@ julia> sum_to(10)
55
julia> s # global
ERROR: UndefVarError: `s` not defined
ERROR: UndefVarError: `s` not defined in `Main`
```

Since `s` is local to the function `sum_to`, calling the function has no effect on the global
Expand Down Expand Up @@ -343,7 +344,7 @@ hello
hello
julia> x
ERROR: UndefVarError: `x` not defined
ERROR: UndefVarError: `x` not defined in `Main`
```

Since the global `x` is not defined when the `for` loop is evaluated, the first clause of the soft
Expand Down Expand Up @@ -408,7 +409,7 @@ julia> code = """
julia> include_string(Main, code)
┌ Warning: Assignment to `s` in soft scope is ambiguous because a global variable by the same name exists: `s` will be treated as a new local. Disambiguate by using `local s` to suppress this warning or `global s` to assign to the existing global variable.
└ @ string:4
ERROR: LoadError: UndefVarError: `s` not defined
ERROR: LoadError: UndefVarError: `s` not defined in local scope
```

Here we use [`include_string`](@ref), to evaluate `code` as though it were the contents of a file.
Expand Down Expand Up @@ -559,7 +560,7 @@ julia> let x = 1, z
println("z: $z") # errors as z has not been assigned yet but is local
end
x: 1, y: -1
ERROR: UndefVarError: `z` not defined
ERROR: UndefVarError: `z` not defined in local scope
```

The assignments are evaluated in order, with each right-hand side evaluated in the scope before
Expand Down
2 changes: 2 additions & 0 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ JL_DLLEXPORT jl_sym_t *jl_thunk_sym;
JL_DLLEXPORT jl_sym_t *jl_foreigncall_sym;
JL_DLLEXPORT jl_sym_t *jl_as_sym;
JL_DLLEXPORT jl_sym_t *jl_global_sym;
JL_DLLEXPORT jl_sym_t *jl_local_sym;
JL_DLLEXPORT jl_sym_t *jl_list_sym;
JL_DLLEXPORT jl_sym_t *jl_dot_sym;
JL_DLLEXPORT jl_sym_t *jl_newvar_sym;
Expand Down Expand Up @@ -322,6 +323,7 @@ void jl_init_common_symbols(void)
jl_opaque_closure_method_sym = jl_symbol("opaque_closure_method");
jl_const_sym = jl_symbol("const");
jl_global_sym = jl_symbol("global");
jl_local_sym = jl_symbol("local");
jl_thunk_sym = jl_symbol("thunk");
jl_toplevel_sym = jl_symbol("toplevel");
jl_dot_sym = jl_symbol(".");
Expand Down
2 changes: 1 addition & 1 deletion src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ static jl_value_t *do_apply(jl_value_t **args, uint32_t nargs, jl_value_t *itera
}
}
if (extra && iterate == NULL) {
jl_undefined_var_error(jl_symbol("iterate"));
jl_undefined_var_error(jl_symbol("iterate"), NULL);
}
// allocate space for the argument array and gc roots for it
// based on our previous estimates
Expand Down
27 changes: 15 additions & 12 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -833,8 +833,10 @@ static const auto jltypeerror_func = new JuliaFunction<>{
};
static const auto jlundefvarerror_func = new JuliaFunction<>{
XSTR(jl_undefined_var_error),
[](LLVMContext &C) { return FunctionType::get(getVoidTy(C),
{PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted)}, false); },
[](LLVMContext &C) {
Type *T = PointerType::get(JuliaType::get_jlvalue_ty(C), AddressSpace::CalleeRooted);
return FunctionType::get(getVoidTy(C), {T, T}, false);
},
get_attrs_noreturn,
};
static const auto jlhasnofield_func = new JuliaFunction<>{
Expand Down Expand Up @@ -1977,7 +1979,7 @@ static jl_returninfo_t get_specsig_function(jl_codectx_t &ctx, Module *M, Value
static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval = -1);
static Value *global_binding_pointer(jl_codectx_t &ctx, jl_module_t *m, jl_sym_t *s,
jl_binding_t **pbnd, bool assign);
static jl_cgval_t emit_checked_var(jl_codectx_t &ctx, Value *bp, jl_sym_t *name, bool isvol, MDNode *tbaa);
static jl_cgval_t emit_checked_var(jl_codectx_t &ctx, Value *bp, jl_sym_t *name, jl_value_t *scope, bool isvol, MDNode *tbaa);
static jl_cgval_t emit_sparam(jl_codectx_t &ctx, size_t i);
static Value *emit_condition(jl_codectx_t &ctx, const jl_cgval_t &condV, const Twine &msg);
static Value *get_current_task(jl_codectx_t &ctx);
Expand Down Expand Up @@ -3099,7 +3101,7 @@ static jl_cgval_t emit_globalref(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *
}
}
// todo: use type info to avoid undef check
return emit_checked_var(ctx, bp, name, false, ctx.tbaa().tbaa_binding);
return emit_checked_var(ctx, bp, name, (jl_value_t*)mod, false, ctx.tbaa().tbaa_binding);
}

static bool emit_globalset(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *sym, const jl_cgval_t &rval_info, AtomicOrdering Order)
Expand Down Expand Up @@ -4892,15 +4894,16 @@ static jl_cgval_t emit_call(jl_codectx_t &ctx, jl_expr_t *ex, jl_value_t *rt, bo

// --- accessing and assigning variables ---

static void undef_var_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *name)
static void undef_var_error_ifnot(jl_codectx_t &ctx, Value *ok, jl_sym_t *name, jl_value_t *scope)
{
++EmittedUndefVarErrors;
BasicBlock *err = BasicBlock::Create(ctx.builder.getContext(), "err", ctx.f);
BasicBlock *ifok = BasicBlock::Create(ctx.builder.getContext(), "ok");
ctx.builder.CreateCondBr(ok, ifok, err);
ctx.builder.SetInsertPoint(err);
ctx.builder.CreateCall(prepare_call(jlundefvarerror_func),
mark_callee_rooted(ctx, literal_pointer_val(ctx, (jl_value_t*)name)));
ctx.builder.CreateCall(prepare_call(jlundefvarerror_func), {
mark_callee_rooted(ctx, literal_pointer_val(ctx, (jl_value_t*)name)),
mark_callee_rooted(ctx, literal_pointer_val(ctx, scope))});
ctx.builder.CreateUnreachable();
ifok->insertInto(ctx.f);
ctx.builder.SetInsertPoint(ifok);
Expand Down Expand Up @@ -4988,7 +4991,7 @@ static Value *global_binding_pointer(jl_codectx_t &ctx, jl_module_t *m, jl_sym_t
return julia_binding_gv(ctx, b);
}

static jl_cgval_t emit_checked_var(jl_codectx_t &ctx, Value *bp, jl_sym_t *name, bool isvol, MDNode *tbaa)
static jl_cgval_t emit_checked_var(jl_codectx_t &ctx, Value *bp, jl_sym_t *name, jl_value_t *scope, bool isvol, MDNode *tbaa)
{
LoadInst *v = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, bp, Align(sizeof(void*)));
setName(ctx.emission_context, v, jl_symbol_name(name) + StringRef(".checked"));
Expand All @@ -4999,7 +5002,7 @@ static jl_cgval_t emit_checked_var(jl_codectx_t &ctx, Value *bp, jl_sym_t *name,
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, tbaa);
ai.decorateInst(v);
}
undef_var_error_ifnot(ctx, ctx.builder.CreateIsNotNull(v), name);
undef_var_error_ifnot(ctx, ctx.builder.CreateIsNotNull(v), name, scope);
return mark_julia_type(ctx, v, true, jl_any_type);
}

Expand All @@ -5025,7 +5028,7 @@ static jl_cgval_t emit_sparam(jl_codectx_t &ctx, size_t i)
sparam = (jl_unionall_t*)sparam->body;
assert(jl_is_unionall(sparam));
}
undef_var_error_ifnot(ctx, isnull, sparam->var->name);
undef_var_error_ifnot(ctx, isnull, sparam->var->name, (jl_value_t*)jl_static_parameter_sym);
return mark_julia_type(ctx, sp, true, jl_any_type);
}

Expand Down Expand Up @@ -5183,7 +5186,7 @@ static jl_cgval_t emit_varinfo(jl_codectx_t &ctx, jl_varinfo_t &vi, jl_sym_t *va
}
if (isnull) {
setName(ctx.emission_context, isnull, jl_symbol_name(varname) + StringRef("_is_null"));
undef_var_error_ifnot(ctx, isnull, varname);
undef_var_error_ifnot(ctx, isnull, varname, (jl_value_t*)jl_local_sym);
}
return v;
}
Expand Down Expand Up @@ -5779,7 +5782,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaidx_
literal_pointer_val(ctx, jl_undefref_exception));
}
else {
undef_var_error_ifnot(ctx, cond, var);
undef_var_error_ifnot(ctx, cond, var, (jl_value_t*)jl_local_sym);
}
return ghostValue(ctx, jl_nothing_type);
}
Expand Down
Loading

0 comments on commit 449c7a2

Please sign in to comment.