From 8688acc03d9230d607a1ad2be0ce69b0a4f28068 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 5 Dec 2017 11:48:57 -0500 Subject: [PATCH] implement getfield overloading New functions `Base.getproperty` and `Base.setproperty!` can be overloaded to change the behavior of `x.f` and `x.f = v`, respectively. fix #16226 (close #16195) fix #1974 --- base/boot.jl | 3 + base/coreimg.jl | 5 +- base/exports.jl | 2 + base/libgit2/gitcredential.jl | 2 +- base/show.jl | 2 +- base/sysimg.jl | 10 +++ src/julia-syntax.scm | 12 +-- src/method.c | 107 ++++++++++++++------------- stdlib/Distributed/src/cluster.jl | 2 +- test/core.jl | 117 ++++++++++++++++-------------- test/reflection.jl | 6 +- test/stacktraces.jl | 4 +- test/vecelement.jl | 2 +- 13 files changed, 150 insertions(+), 124 deletions(-) diff --git a/base/boot.jl b/base/boot.jl index 8ebcfb4c10af9..2d293d2d9ab3c 100644 --- a/base/boot.jl +++ b/base/boot.jl @@ -153,6 +153,9 @@ export # constants nothing, Main +const getproperty = getfield +const setproperty! = setfield! + abstract type Number end abstract type Real <: Number end abstract type AbstractFloat <: Real end diff --git a/base/coreimg.jl b/base/coreimg.jl index 11e29947b5be2..750386a7c7368 100644 --- a/base/coreimg.jl +++ b/base/coreimg.jl @@ -1,9 +1,12 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license -Main.Core.eval(Main.Core, :(baremodule Inference +getfield(getfield(Main, :Core), :eval)(getfield(Main, :Core), :(baremodule Inference using Core.Intrinsics import Core: print, println, show, write, unsafe_write, STDOUT, STDERR +const getproperty = getfield +const setproperty! = setfield! + ccall(:jl_set_istopmod, Void, (Any, Bool), Inference, false) eval(x) = Core.eval(Inference, x) diff --git a/base/exports.jl b/base/exports.jl index abfa7088b1398..0f05ad8a24c7d 100644 --- a/base/exports.jl +++ b/base/exports.jl @@ -898,6 +898,8 @@ export # types convert, + # getproperty, + # setproperty!, fieldoffset, fieldname, fieldnames, diff --git a/base/libgit2/gitcredential.jl b/base/libgit2/gitcredential.jl index 2eb1b69471371..dc5ca4908323c 100644 --- a/base/libgit2/gitcredential.jl +++ b/base/libgit2/gitcredential.jl @@ -116,7 +116,7 @@ function Base.read!(io::IO, cred::GitCredential) # https://git-scm.com/docs/git-credential#git-credential-codeurlcode copy!(cred, parse(GitCredential, value)) else - setfield!(cred, Symbol(key), String(value)) + Base.setproperty!(cred, Symbol(key), String(value)) end end diff --git a/base/show.jl b/base/show.jl index c7c515bf6efb0..87880528d18a0 100644 --- a/base/show.jl +++ b/base/show.jl @@ -146,7 +146,7 @@ function show_default(io::IO, @nospecialize(x)) if !isdefined(x, f) print(io, undef_ref_str) else - show(recur_io, getfield(x, f)) + show(recur_io, getfield(x, i)) end if i < nf print(io, ", ") diff --git a/base/sysimg.jl b/base/sysimg.jl index 80b89d572ab24..5b596c1ce817c 100644 --- a/base/sysimg.jl +++ b/base/sysimg.jl @@ -5,6 +5,16 @@ baremodule Base using Core.Intrinsics ccall(:jl_set_istopmod, Void, (Any, Bool), Base, true) +getproperty(x, f::Symbol) = getfield(x, f) +setproperty!(x, f::Symbol, v) = setfield!(x, f, convert(fieldtype(typeof(x), f), v)) + +# Try to help prevent users from shooting them-selves in the foot +# with ambiguities by defining a few common and critical operations +getproperty(x::Module, f::Symbol) = getfield(x, f) +setproperty!(x::Module, f::Symbol, v) = setfield!(x, f, v) +getproperty(x::Type, f::Symbol) = getfield(x, f) +setproperty!(x::Type, f::Symbol, v) = setfield!(x, f, v) + function include(mod::Module, path::AbstractString) local result if INCLUDE_STATE === 1 diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index e2e940e362e6c..b68493a1dfb90 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -1752,7 +1752,7 @@ (if (and (pair? e) (eq? (car e) '|.|)) (let ((f (cadr e)) (x (caddr e))) (cond ((or (eq? (car x) 'quote) (eq? (car x) 'inert) (eq? (car x) '$)) - `(call (core getfield) ,f ,x)) + `(call (top getproperty) ,f ,x)) ((eq? (car x) 'tuple) (make-fuse f (cdr x))) (else @@ -2039,10 +2039,12 @@ ,.(if (eq? aa a) '() `((= ,aa ,(expand-forms a)))) ,.(if (eq? bb b) '() `((= ,bb ,(expand-forms b)))) ,.(if (eq? rr rhs) '() `((= ,rr ,(expand-forms rhs)))) - (call (core setfield!) ,aa ,bb - (call (top convert) - (call (core fieldtype) (call (core typeof) ,aa) ,bb) - ,rr)) + (call (top setproperty!) ,aa ,bb + (if (call (top ===) (top setproperty!) (core setfield!)) + (call (top convert) + (call (core fieldtype) (call (core typeof) ,aa) ,bb) + ,rr) + ,rr)) (unnecessary ,rr))))) ((tuple) ;; multiple assignment diff --git a/src/method.c b/src/method.c index 8c734334d5dd4..e956abdc76b55 100644 --- a/src/method.c +++ b/src/method.c @@ -41,60 +41,7 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve // ignore these } else { - size_t nargs = jl_expr_nargs(e); - if (e->head == call_sym && nargs > 0) { - jl_value_t *fe = jl_exprarg(e, 0); - if (jl_is_globalref(fe) && jl_binding_resolved_p(jl_globalref_mod(fe), jl_globalref_name(fe))) { - // look at some known called functions - jl_binding_t *b = jl_get_binding(jl_globalref_mod(fe), jl_globalref_name(fe)); - jl_value_t *f = b && b->constp ? b->value : NULL; - if (f == jl_builtin_getfield && nargs == 3 && - jl_is_quotenode(jl_exprarg(e, 2)) && module != NULL) { - // replace getfield(module_expr, :sym) with GlobalRef - jl_value_t *s = jl_fieldref(jl_exprarg(e, 2), 0); - if (jl_is_symbol(s)) { - jl_value_t *me = jl_exprarg(e, 1); - jl_module_t *me_mod = NULL; - jl_sym_t *me_sym = NULL; - if (jl_is_globalref(me)) { - me_mod = jl_globalref_mod(me); - me_sym = jl_globalref_name(me); - } - else if (jl_is_symbol(me) && jl_binding_resolved_p(module, (jl_sym_t*)me)) { - me_mod = module; - me_sym = (jl_sym_t*)me; - } - if (me_mod && me_sym) { - jl_binding_t *b = jl_get_binding(me_mod, me_sym); - if (b && b->constp) { - jl_value_t *m = b->value; - if (m && jl_is_module(m)) { - return jl_module_globalref((jl_module_t*)m, (jl_sym_t*)s); - } - } - } - } - } - else if (f == jl_builtin_tuple) { - size_t j; - for (j = 1; j < nargs; j++) { - if (!jl_is_quotenode(jl_exprarg(e,j))) - break; - } - if (j == nargs) { - jl_value_t *val = NULL; - JL_TRY { - val = jl_interpret_toplevel_expr_in(module, (jl_value_t*)e, NULL, sparam_vals); - } - JL_CATCH { - } - if (val) - return val; - } - } - } - } - size_t i = 0; + size_t i = 0, nargs = jl_array_len(e->args); if (e->head == foreigncall_sym) { JL_NARGSV(ccall method definition, 5); // (fptr, rt, at, cc, narg) jl_value_t *rt = jl_exprarg(e, 1); @@ -139,6 +86,58 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve // TODO: this should be making a copy, not mutating the source jl_exprargset(e, i, resolve_globals(jl_exprarg(e, i), module, sparam_vals, binding_effects)); } + if (e->head == call_sym && jl_expr_nargs(e) == 3 && + jl_is_globalref(jl_exprarg(e, 0)) && + jl_is_globalref(jl_exprarg(e, 1)) && + jl_is_quotenode(jl_exprarg(e, 2))) { + // replace module_expr.sym with GlobalRef(module, sym) + // for expressions pattern-matching to `getproperty(module_expr, :sym)` in a top-module + // (this is expected to help inference performance) + // TODO: this was broken by linear-IR + jl_value_t *s = jl_fieldref(jl_exprarg(e, 2), 0); + jl_value_t *me = jl_exprarg(e, 1); + jl_value_t *fe = jl_exprarg(e, 0); + jl_module_t *fe_mod = jl_globalref_mod(fe); + jl_sym_t *fe_sym = jl_globalref_name(fe); + jl_module_t *me_mod = jl_globalref_mod(me); + jl_sym_t *me_sym = jl_globalref_name(me); + if (fe_mod->istopmod && !strcmp(jl_symbol_name(fe_sym), "getproperty") && jl_is_symbol(s)) { + if (jl_binding_resolved_p(me_mod, me_sym)) { + jl_binding_t *b = jl_get_binding(me_mod, me_sym); + if (b && b->constp && b->value && jl_is_module(b->value)) { + return jl_module_globalref((jl_module_t*)b->value, (jl_sym_t*)s); + } + } + } + } + if (e->head == call_sym && nargs > 0 && + jl_is_globalref(jl_exprarg(e, 0))) { + // TODO: this hack should be deleted once llvmcall is fixed + jl_value_t *fe = jl_exprarg(e, 0); + jl_module_t *fe_mod = jl_globalref_mod(fe); + jl_sym_t *fe_sym = jl_globalref_name(fe); + if (jl_binding_resolved_p(fe_mod, fe_sym)) { + // look at some known called functions + jl_binding_t *b = jl_get_binding(fe_mod, fe_sym); + if (b && b->constp && b->value == jl_builtin_tuple) { + size_t j; + for (j = 1; j < nargs; j++) { + if (!jl_is_quotenode(jl_exprarg(e, j))) + break; + } + if (j == nargs) { + jl_value_t *val = NULL; + JL_TRY { + val = jl_interpret_toplevel_expr_in(module, (jl_value_t*)e, NULL, sparam_vals); + } + JL_CATCH { + } + if (val) + return val; + } + } + } + } } } return expr; diff --git a/stdlib/Distributed/src/cluster.jl b/stdlib/Distributed/src/cluster.jl index 2f21b6e7809f1..d5bf8ef7859e9 100644 --- a/stdlib/Distributed/src/cluster.jl +++ b/stdlib/Distributed/src/cluster.jl @@ -473,7 +473,7 @@ function launch_n_additional_processes(manager, frompid, fromconfig, cnt, launch wconfig = WorkerConfig() for x in [:host, :tunnel, :sshflags, :exeflags, :exename, :enable_threaded_blas] - setfield!(wconfig, x, getfield(fromconfig, x)) + Base.setproperty!(wconfig, x, Base.getproperty(fromconfig, x)) end wconfig.bind_addr = bind_addr wconfig.port = port diff --git a/test/core.jl b/test/core.jl index d00edb0d35e5a..6aac4d4ea5320 100644 --- a/test/core.jl +++ b/test/core.jl @@ -997,19 +997,21 @@ end let local z = complex(3, 4) - v = Int[0,0] - for i=1:2 + v = Int[0, 0] + for i = 1:2 v[i] = getfield(z, i) end - @test v == [3,4] - @test_throws BoundsError getfield(z, -1) - @test_throws BoundsError getfield(z, 0) - @test_throws BoundsError getfield(z, 3) + @test v == [3, 4] + @test_throws BoundsError(z, -1) getfield(z, -1) + @test_throws BoundsError(z, 0) getfield(z, 0) + @test_throws BoundsError(z, 3) getfield(z, 3) strct = LoadError("yofile", 0, "bad") - @test_throws BoundsError getfield(strct, 10) - @test_throws ErrorException setfield!(strct, 0, "") - @test_throws ErrorException setfield!(strct, 4, "") + @test nfields(strct) == 3 # sanity test + @test_throws BoundsError(strct, 10) getfield(strct, 10) + @test_throws ErrorException("type LoadError is immutable") setfield!(strct, 0, "") + @test_throws ErrorException("type LoadError is immutable") setfield!(strct, 4, "") + @test_throws ErrorException("type is immutable") setfield!(strct, :line, 0) @test strct.file == "yofile" @test strct.line == 0 @test strct.error == "bad" @@ -1018,15 +1020,17 @@ let @test getfield(strct, 3) == "bad" mstrct = TestMutable("melm", 1, nothing) - setfield!(mstrct, 2, 8) + Base.setproperty!(mstrct, :line, 8.0) @test mstrct.line == 8 + @test_throws TypeError(:setfield!, "", Int, 8.0) setfield!(mstrct, :line, 8.0) + @test_throws TypeError(:setfield!, "", Int, 8.0) setfield!(mstrct, 2, 8.0) setfield!(mstrct, 3, "hi") @test mstrct.error == "hi" setfield!(mstrct, 1, "yo") @test mstrct.file == "yo" - @test_throws BoundsError getfield(mstrct, 10) - @test_throws BoundsError setfield!(mstrct, 0, "") - @test_throws BoundsError setfield!(mstrct, 4, "") + @test_throws BoundsError(mstrct, 10) getfield(mstrct, 10) + @test_throws BoundsError(mstrct, 0) setfield!(mstrct, 0, "") + @test_throws BoundsError(mstrct, 4) setfield!(mstrct, 4, "") end # allow typevar in Union to match as long as the arguments contain @@ -2175,10 +2179,9 @@ g7652() = fieldtype(DataType, :types) h7652() = setfield!(a7652, 1, 2) h7652() @test a7652.a == 2 -# commented out due to issue #16195: setfield! does not perform conversions -# i7652() = setfield!(a7652, 1, 3.0) -# i7652() -# @test a7652.a == 3 +i7652() = Base.setproperty!(a7652, :a, 3.0) +i7652() +@test a7652.a == 3 # issue #7679 @test map(f->f(), Any[ ()->i for i=1:3 ]) == Any[1,2,3] @@ -2358,49 +2361,53 @@ let end # pull request #9534 -@test try; a,b,c = 1,2; catch ex; (ex::BoundsError).a === (1,2) && ex.i == 3; end -# @test try; [][]; catch ex; isempty((ex::BoundsError).a::Array{Any,1}) && ex.i == (1,); end # TODO: Re-enable after PLI -@test try; [][1,2]; catch ex; isempty((ex::BoundsError).a::Array{Any,1}) && ex.i == (1,2); end -@test try; [][10]; catch ex; isempty((ex::BoundsError).a::Array{Any,1}) && ex.i == (10,); end -f9534a() = (a=1+2im; getfield(a, -100)) -f9534a(x) = (a=1+2im; getfield(a, x)) -@test try; f9534a() catch ex; (ex::BoundsError).a === 1+2im && ex.i == -100; end -@test try; f9534a(3) catch ex; (ex::BoundsError).a === 1+2im && ex.i == 3; end -f9534b() = (a=(1,2.,""); a[5]) -f9534b(x) = (a=(1,2.,""); a[x]) -@test try; f9534b() catch ex; (ex::BoundsError).a == (1,2.,"") && ex.i == 5; end -@test try; f9534b(4) catch ex; (ex::BoundsError).a == (1,2.,"") && ex.i == 4; end -f9534c() = (a=(1,2.); a[3]) -f9534c(x) = (a=(1,2.); a[x]) -@test try; f9534c() catch ex; (ex::BoundsError).a === (1,2.) && ex.i == 3; end -@test try; f9534c(0) catch ex; (ex::BoundsError).a === (1,2.) && ex.i == 0; end -f9534d() = (a=(1,2,4,6,7); a[7]) -f9534d(x) = (a=(1,2,4,6,7); a[x]) -@test try; f9534d() catch ex; (ex::BoundsError).a === (1,2,4,6,7) && ex.i == 7; end -@test try; f9534d(-1) catch ex; (ex::BoundsError).a === (1,2,4,6,7) && ex.i == -1; end -f9534e(x) = (a=IOBuffer(); setfield!(a, x, 3)) -@test try; f9534e(-2) catch ex; isa((ex::BoundsError).a,Base.IOBuffer) && ex.i == -2; end -f9534f() = (a=IOBuffer(); getfield(a, -2)) -f9534f(x) = (a=IOBuffer(); getfield(a, x)) -@test try; f9534f() catch ex; isa((ex::BoundsError).a,Base.IOBuffer) && ex.i == -2; end -@test try; f9534f(typemin(Int)+2) catch ex; isa((ex::BoundsError).a,Base.IOBuffer) && ex.i == typemin(Int)+2; end +@test_throws BoundsError((1, 2), 3) begin; a, b, c = 1, 2; end +let a = [] + @test_broken try; a[]; catch ex; (ex::BoundsError).a === a && ex.i == (1,); end # TODO: Re-enable after PLI + @test_throws BoundsError(a, (1, 2)) a[1, 2] + @test_throws BoundsError(a, (10,)) a[10] +end +f9534a() = (a = 1 + 2im; getfield(a, -100)) +f9534a(x) = (a = 1 + 2im; getfield(a, x)) +@test_throws BoundsError(1 + 2im, -100) f9534a() +@test_throws BoundsError(1 + 2im, 3) f9534a(3) +f9534b() = (a = (1, 2., ""); a[5]) +f9534b(x) = (a = (1, 2., ""); a[x]) +@test_throws BoundsError((1, 2., ""), 5) f9534b() +@test_throws BoundsError((1, 2., ""), 4) f9534b(4) +f9534c() = (a = (1, 2.); a[3]) +f9534c(x) = (a = (1, 2.); a[x]) +@test_throws BoundsError((1, 2.), 3) f9534c() +@test_throws BoundsError((1, 2.), 0) f9534c(0) +f9534d() = (a = (1, 2, 4, 6, 7); a[7]) +f9534d(x) = (a = (1, 2, 4, 6, 7); a[x]) +@test_throws BoundsError((1, 2, 4, 6, 7), 7) f9534d() +@test_throws BoundsError((1, 2, 4, 6, 7), -1) f9534d(-1) +let a = IOBuffer() + f9534e(x) = setfield!(a, x, 3) + @test_throws BoundsError(a, -2) f9534e(-2) + f9534f() = getfield(a, -2) + f9534f(x) = getfield(a, x) + @test_throws BoundsError(a, -2) f9534f() + @test_throws BoundsError(a, typemin(Int) + 2) f9534f(typemin(Int) + 2) +end x9634 = 3 -@test try; getfield(1+2im, x9634); catch ex; (ex::BoundsError).a === 1+2im && ex.i == 3; end -@test try; throw(BoundsError()) catch ex; !isdefined((ex::BoundsError), :a) && !isdefined((ex::BoundsError), :i); end -@test try; throw(BoundsError(Int)) catch ex; (ex::BoundsError).a == Int && !isdefined((ex::BoundsError), :i); end -@test try; throw(BoundsError(Int, typemin(Int))) catch ex; (ex::BoundsError).a == Int && (ex::BoundsError).i == typemin(Int); end -@test try; throw(BoundsError(Int, (:a,))) catch ex; (ex::BoundsError).a == Int && (ex::BoundsError).i == (:a,); end -f9534g(a,b,c...) = c[0] -@test try; f9534g(1,2,3,4,5,6) catch ex; (ex::BoundsError).a === (3,4,5,6) && ex.i == 0; end -f9534h(a,b,c...) = c[a] -@test f9534h(4,2,3,4,5,6) == 6 -@test try; f9534h(5,2,3,4,5,6) catch ex; (ex::BoundsError).a === (3,4,5,6) && ex.i == 5; end +@test_throws BoundsError(1 + 2im, 3) getfield(1 + 2im, x9634) +@test try; throw(BoundsError()); catch ex; !isdefined((ex::BoundsError), :a) && !isdefined((ex::BoundsError), :i); end +@test try; throw(BoundsError(Int)); catch ex; (ex::BoundsError).a == Int && !isdefined((ex::BoundsError), :i); end +@test_throws BoundsError(Int, typemin(Int)) throw(BoundsError(Int, typemin(Int))) +@test_throws BoundsError(Int, (:a,)) throw(BoundsError(Int, (:a,))) +f9534g(a, b, c...) = c[0] +@test_throws BoundsError((3, 4, 5, 6), 0) f9534g(1, 2, 3, 4, 5, 6) +f9534h(a, b, c...) = c[a] +@test f9534h(4, 2, 3, 4, 5, 6) == 6 +@test_throws BoundsError((3, 4, 5, 6), 5) f9534h(5, 2, 3, 4, 5, 6) # issue #7978, comment 332352438 f7978a() = 1 -@test try; a, b = f7978a() catch ex; (ex::BoundsError).a == 1 && ex.i == 2; end +@test_throws BoundsError(1, 2) begin; a, b = f7978a(); end f7978b() = 1, 2 -@test try; a, b, c = f7978b() catch ex; (ex::BoundsError).a == (1, 2) && ex.i == 3; end +@test_throws BoundsError((1, 2), 3) begin; a, b, c = f7978b(); end # issue #9535 counter9535 = 0 diff --git a/test/reflection.jl b/test/reflection.jl index e79374018f4a5..b1e458a53ac81 100644 --- a/test/reflection.jl +++ b/test/reflection.jl @@ -505,14 +505,14 @@ test_typed_ast_printing(g15714, Tuple{Vector{Float32}}, @test used_dup_var_tested15714 @test used_unique_var_tested15714 -let li = typeof(getfield).name.mt.cache.func::Core.MethodInstance, +let li = typeof(fieldtype).name.mt.cache.func::Core.MethodInstance, lrepr = string(li), mrepr = string(li.def), lmime = stringmime("text/plain", li), mmime = stringmime("text/plain", li.def) - @test lrepr == lmime == "MethodInstance for getfield(...)" - @test mrepr == mmime == "getfield(...) in Core" + @test lrepr == lmime == "MethodInstance for fieldtype(...)" + @test mrepr == mmime == "fieldtype(...) in Core" end diff --git a/test/stacktraces.jl b/test/stacktraces.jl index 633e8c7fee947..ea646649203b7 100644 --- a/test/stacktraces.jl +++ b/test/stacktraces.jl @@ -109,10 +109,10 @@ let src = Meta.lower(Main, quote let x = 1 end end).args[1]::CodeInfo, repr = string(sf) @test repr == "Toplevel MethodInstance thunk at b:3" end -let li = typeof(getfield).name.mt.cache.func::Core.MethodInstance, +let li = typeof(fieldtype).name.mt.cache.func::Core.MethodInstance, sf = StackFrame(:a, :b, 3, li, false, false, 0), repr = string(sf) - @test repr == "getfield(...) at b:3" + @test repr == "fieldtype(...) at b:3" end let ctestptr = cglobal((:ctest, "libccalltest")), diff --git a/test/vecelement.jl b/test/vecelement.jl index e9c5baa029de5..30ce82cee05b9 100644 --- a/test/vecelement.jl +++ b/test/vecelement.jl @@ -90,7 +90,7 @@ const _llvmtypes = Dict{DataType, String}( """ return quote Base.@_inline_meta - Base.llvmcall($exp, Vec{$N, $T}, Tuple{Vec{$N, $T}, Vec{$N, $T}}, x, y) + Core.getfield(Base, :llvmcall)($exp, Vec{$N, $T}, Tuple{Vec{$N, $T}, Vec{$N, $T}}, x, y) end end