Skip to content

Commit

Permalink
Make @deprecate_binding work with --depwarn cmdline flags
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakebolewski
jakebolewski committed Sep 24, 2015
1 parent f6665b1 commit 31a0c55
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/module.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,17 +482,25 @@ DLLEXPORT int jl_is_binding_deprecated(jl_module_t *m, jl_sym_t *var)
void jl_binding_deprecation_warning(jl_binding_t *b)
{
if (b->deprecated && jl_options.depwarn) {
if (jl_options.depwarn != JL_OPTIONS_DEPWARN_ERROR)
jl_printf(JL_STDERR, "WARNING: ");
if (b->owner)
jl_printf(JL_STDERR, "WARNING: %s.%s is deprecated", b->owner->name->name, b->name->name);
jl_printf(JL_STDERR, "%s.%s is deprecated", b->owner->name->name, b->name->name);
else
jl_printf(JL_STDERR, "WARNING: %s is deprecated", b->name->name);
jl_printf(JL_STDERR, "%s is deprecated", b->name->name);
jl_value_t *v = b->value;
if (v && (jl_is_type(v) || (jl_is_function(v) && jl_is_gf(v)))) {
jl_printf(JL_STDERR, ", use ");
jl_static_show(JL_STDERR, v);
jl_printf(JL_STDERR, " instead");
}
jl_printf(JL_STDERR, ".\n");
if (jl_options.depwarn == JL_OPTIONS_DEPWARN_ERROR) {
if (b->owner)
jl_errorf("deprecated binding: %s.%s", b->owner->name->name, b->name->name);
else
jl_errorf("deprecated binding: %s", b->name->name);
}
}
}

Expand Down
33 changes: 33 additions & 0 deletions test/cmdlineargs.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,39 @@ let exename = `$(joinpath(JULIA_HOME, Base.julia_exename())) --precompiled=yes`
foo()
" --depwarn=error`)

# test deprecated bindings
let code = """
module Foo
import Base: @deprecate_binding
const NotDeprecated = true
@deprecate_binding Deprecated NotDeprecated
end
Foo.Deprecated
"""

@test !success(`$exename -E "$code" --depwarn=error`)

@unix_only let out = Pipe(),
proc = spawn(pipeline(`$exename -E "$code" --depwarn=yes`, stderr=out))

wait(proc)

This comment has been minimized.

Sign in to view

Copy link
@samoconnor

samoconnor Feb 15, 2016

Contributor

Deadlock here ? @vtjnash ? @jakebolewski ?
No-one is reading from the subprocess's stderr pipe, so the subprocess is blocked writing to the pipe.

Need something like:

out  = Pipe()
proc = spawn(pipeline(`$exename -E "$code" --depwarn=yes`, stderr=out))

result = UInt8[]
@async append!(result, read(out))
result = UTF8String(result)
wait(proc)

( @tkelman: related to ?https://github.com/JuliaLang/julia/blob/master/test/cmdlineargs.jl#L168 )

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Feb 15, 2016

Contributor

commenting on a 5 month old commit by a not currently active author is not a good way to ensure a response to an issue

This comment has been minimized.

Sign in to view

Copy link
@vtjnash

vtjnash Feb 15, 2016

Member

yes, that analysis is correct. the task should block waiting on readstring(out), then confirm success(proc) instead of the reverse. i've occasionally fixed other tests to correct this, but hadn't noticed this one. can you submit a PR to fix?

unfortunately, I don't know any better way to implement this facet of IO – maybe switching to a io-threadpool could help, but maybe that'll require other design work too.

This comment has been minimized.

Sign in to view

Copy link
@samoconnor

samoconnor Feb 15, 2016

Contributor

Hi @vtjnash I have a branch with this fix, but it is currently blocked by #15088 (whoops, which I see you have just addressed, thx)

This comment has been minimized.

Sign in to view

Copy link
@samoconnor

samoconnor Feb 16, 2016

Contributor

PR: #15091
close(out.in)
@test success(proc)
@test readchomp(out) == "WARNING: Foo.Deprecated is deprecated."
end

@unix_only let out = Pipe(),
proc = spawn(pipeline(`$exename -E "$code" --depwarn=no`, stderr=out))

wait(proc)
close(out.in)
@test success(proc)
@test isempty(readall(out))
end
end

# --inline
@test readchomp(`$exename -E "Bool(Base.JLOptions().can_inline)"`) == "true"
@test readchomp(`$exename --inline=yes -E "Bool(Base.JLOptions().can_inline)"`) == "true"
Expand Down

0 comments on commit 31a0c55

Please sign in to comment.