Skip to content

Commit

Permalink
add verbose UndefVarError messages
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 Oct 31, 2023
1 parent 5ae88f5 commit 434809b
Show file tree
Hide file tree
Showing 32 changed files with 183 additions and 97 deletions.
3 changes: 3 additions & 0 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,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 @@ -168,6 +168,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 @@ -320,6 +321,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 @@ -788,8 +788,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 @@ -1882,7 +1884,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 @@ -3005,7 +3007,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 @@ -4798,15 +4800,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 @@ -4894,7 +4897,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 @@ -4905,7 +4908,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 @@ -4931,7 +4934,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 @@ -5089,7 +5092,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 @@ -5673,7 +5676,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 434809b

Please sign in to comment.