Skip to content

Commit

Permalink
Forward all frontend depwarns to the logging system
Browse files Browse the repository at this point in the history
Forward all frontend depwarn messages through to the julia logging
system for consistency of formatting and dispatch.  Deciding whether a
depwarn should appear as an thrown error is also now handled outside
flisp to keep this decision in one place where possible.

Detail:

* Ensure that deprecation-message forwards to the logging system.  Also
  include file and line number metadata
* Remove jl_parse_depwarn(), flisp *depwarn* and related scheme code as
  this can be handled on the julia side.
* Rename syntax_deprecation_warnings(false) to
  without_syntax_deprecations, as this was only ever used to turn syntax
  deprecations off.
* Ensure that all lowering depwarns with existing line number
  information get this passed through as accessible metadata rather than
  as a string.
* Use distinct functions for depwarns coming from the parser vs
  lowering, as these get line number information in a different way.

TODO:
* Figure out a decent `id` for the syntax and lowering depwarn messages
* Make without_syntax_deprecations filter only depwarn messages, rather
  than everything!
  • Loading branch information
c42f committed Sep 30, 2017
1 parent 2482202 commit ba866dc
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 141 deletions.
14 changes: 4 additions & 10 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -185,16 +185,10 @@ function eval_user_input(@nospecialize(ast), show_value)
isa(STDIN,TTY) && println()
end

syntax_deprecation_warnings(warn::Bool) =
ccall(:jl_parse_depwarn, Cint, (Cint,), warn) == 1

function syntax_deprecation_warnings(f::Function, warn::Bool)
prev = syntax_deprecation_warnings(warn)
try
f()
finally
syntax_deprecation_warnings(prev)
end
function without_syntax_deprecations(f::Function)
# Turn off all logging
# TODO: Disable only depwarns, but in a clean way.
with_logger(f, Logging.NullLogger())
end

function parse_input_line(s::String; filename::String="none")
Expand Down
20 changes: 18 additions & 2 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,12 @@ macro deprecate(old, new, ex=true)
end
end

# NB: keep in sync with JL_OPTIONS_DEPWARN_THROW
depwarn_should_throw(opts) = opts.depwarn == typemax(Int32)

function depwarn(msg, funcsym)
opts = JLOptions()
if opts.depwarn == typemax(Int32)
if depwarn_should_throw(opts)
throw(ErrorException(msg))
end
deplevel = Logging.LogLevel(opts.depwarn)
Expand All @@ -74,7 +77,7 @@ function depwarn(msg, funcsym)
_module=begin
bt = backtrace()
frame, caller = firstcaller(bt, funcsym)
# FIXME - The isnull handling isn't great here.
# FIXME - Which module should a null here be attributed to?
!isnull(caller.linfo) ? caller.linfo.value.def.module : Base
end,
_file=caller.file,
Expand All @@ -87,6 +90,19 @@ function depwarn(msg, funcsym)
nothing
end

# Emit deprecated syntax warning. This function exists to be called from
# the julia parser (see flisp julia-syntax-depwarn) and in that context any
# exceptions will be ignored.
function syntax_depwarn(msg, file, line)
opts = JLOptions()
if depwarn_should_throw(opts)
return 2 # Will throw error on parser side to simplify marshalling
end
deplevel = Logging.LogLevel(opts.depwarn)
@logmsg deplevel msg _file=file _line=line _group=:depwarn
return 0
end

firstcaller(bt::Array{Ptr{Void},1}, funcsym::Symbol) = firstcaller(bt, (funcsym,))
function firstcaller(bt::Array{Ptr{Void},1}, funcsyms)
# Identify the calling line
Expand Down
2 changes: 1 addition & 1 deletion base/docs/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ function helpmode(io::IO, line::AbstractString)
# keyword such as `function` would throw a parse error due to the missing `end`.
Symbol(line)
else
x = Base.syntax_deprecation_warnings(false) do
x = Base.without_syntax_deprecations() do
parse(line, raise = false)
end
# Retrieving docs for macros requires us to make a distinction between the text
Expand Down
4 changes: 2 additions & 2 deletions base/repl/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ end
LineEdit.reset_state(hist::REPLHistoryProvider) = history_reset_state(hist)

function return_callback(s)
ast = Base.syntax_deprecation_warnings(false) do
ast = Base.without_syntax_deprecations() do
Base.parse_input_line(String(take!(copy(LineEdit.buffer(s)))))
end
if !isa(ast, Expr) || (ast.head != :continue && ast.head != :incomplete)
Expand Down Expand Up @@ -894,7 +894,7 @@ function setup_interface(
continue
end
end
ast, pos = Base.syntax_deprecation_warnings(false) do
ast, pos = Base.without_syntax_deprecations() do
Base.parse(input, oldpos, raise=false)
end
if (isa(ast, Expr) && (ast.head == :error || ast.head == :continue || ast.head == :incomplete)) ||
Expand Down
6 changes: 3 additions & 3 deletions base/repl/REPLCompletions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function complete_symbol(sym, ffunc)
# Find module
lookup_name, name = rsplit(sym, ".", limit=2)

ex = Base.syntax_deprecation_warnings(false) do
ex = Base.without_syntax_deprecations() do
parse(lookup_name, raise=false)
end

Expand Down Expand Up @@ -460,7 +460,7 @@ end
function completions(string, pos)
# First parse everything up to the current position
partial = string[1:pos]
inc_tag = Base.syntax_deprecation_warnings(false) do
inc_tag = Base.without_syntax_deprecations() do
Base.incomplete_tag(parse(partial, raise=false))
end

Expand Down Expand Up @@ -508,7 +508,7 @@ function completions(string, pos)

if inc_tag == :other && should_method_complete(partial)
frange, method_name_end = find_start_brace(partial)
ex = Base.syntax_deprecation_warnings(false) do
ex = Base.without_syntax_deprecations() do
parse(partial[frange] * ")", raise=false)
end
if isa(ex, Expr) && ex.head==:call
Expand Down
67 changes: 34 additions & 33 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,16 +168,47 @@ value_t fl_julia_scalar(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
return fl_ctx->F;
}

static jl_value_t *scm_to_julia_(fl_context_t *fl_ctx, value_t e, jl_module_t *mod);


value_t fl_julia_depwarn(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
{
if (nargs < 1)
lerror(fl_ctx, fl_ctx->ArgError, "julia-depwarn: too few arguments");
static jl_value_t *depwarn_func = NULL;
if (!depwarn_func && jl_base_module)
depwarn_func = jl_get_global(jl_base_module, jl_symbol("syntax_depwarn"));
if (!depwarn_func) {
// No julia function found - use fallbacks in flisp
return fixnum(jl_options.depwarn == JL_OPTIONS_DEPWARN_THROW ? 2 : 1);
}
value_t ret = fixnum(0);
jl_value_t **depwarn_args;
JL_GC_PUSHARGS(depwarn_args, nargs+2);
JL_TRY {
depwarn_args[0] = depwarn_func;
for (int i = 0; i < nargs; ++i)
depwarn_args[i+1] = scm_to_julia_(fl_ctx, args[i], NULL);
jl_value_t* jlret = jl_apply(depwarn_args, nargs+1);
depwarn_args[nargs+1] = jlret;
ret = julia_to_scm(fl_ctx, jlret);
}
JL_CATCH {
// Internal error on julia side - assume logging failed
ret = fixnum(1);
}
JL_GC_POP();
return ret;
}

static const builtinspec_t julia_flisp_ast_ext[] = {
{ "defined-julia-global", fl_defined_julia_global },
{ "current-julia-module-counter", fl_current_module_counter },
{ "julia-scalar?", fl_julia_scalar },
{ "julia-depwarn", fl_julia_depwarn },
{ NULL, NULL }
};

static int jl_parse_deperror(fl_context_t *fl_ctx, int err);
static int jl_parse_depwarn_(fl_context_t *fl_ctx, int warn);

static void jl_init_ast_ctx(jl_ast_context_t *ast_ctx)
{
fl_context_t *fl_ctx = &ast_ctx->fl;
Expand All @@ -201,12 +232,6 @@ static void jl_init_ast_ctx(jl_ast_context_t *ast_ctx)
ctx->slot_sym = symbol(fl_ctx, "slot");
ctx->task = NULL;
ctx->module = NULL;

// Enable / disable syntax deprecation warnings
if (jl_options.depwarn == JL_OPTIONS_DEPWARN_THROW)
jl_parse_deperror(fl_ctx, 1);
else
jl_parse_depwarn_(fl_ctx, (int)jl_options.depwarn);
}

// There should be no GC allocation while holding this lock
Expand Down Expand Up @@ -397,8 +422,6 @@ static jl_sym_t *scmsym_to_julia(fl_context_t *fl_ctx, value_t s)
return jl_symbol(symbol_name(fl_ctx, s));
}

static jl_value_t *scm_to_julia_(fl_context_t *fl_ctx, value_t e, jl_module_t *mod);

static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mod)
{
jl_value_t *v = NULL;
Expand Down Expand Up @@ -828,28 +851,6 @@ JL_DLLEXPORT jl_value_t *jl_load_file_string(const char *text, size_t len,
return jl_parse_eval_all(filename, text, len, inmodule);
}

JL_DLLEXPORT int jl_parse_depwarn(int warn)
{
jl_ast_context_t *ctx = jl_ast_ctx_enter();
int res = jl_parse_depwarn_(&ctx->fl, warn);
jl_ast_ctx_leave(ctx);
return res;
}

static int jl_parse_depwarn_(fl_context_t *fl_ctx, int warn)
{
value_t prev = fl_applyn(fl_ctx, 1, symbol_value(symbol(fl_ctx, "jl-parser-depwarn")),
warn ? fl_ctx->T : fl_ctx->F);
return prev == fl_ctx->T ? 1 : 0;
}

static int jl_parse_deperror(fl_context_t *fl_ctx, int err)
{
value_t prev = fl_applyn(fl_ctx, 1, symbol_value(symbol(fl_ctx, "jl-parser-deperror")),
err ? fl_ctx->T : fl_ctx->F);
return prev == fl_ctx->T ? 1 : 0;
}

// returns either an expression or a thunk
jl_value_t *jl_call_scm_on_ast(const char *funcname, jl_value_t *expr, jl_module_t *inmodule)
{
Expand Down
47 changes: 34 additions & 13 deletions src/jlfrontend.scm
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
error incomplete))
(and (eq? (car e) 'global) (every symbol? (cdr e))))))
(if (eq? e '_)
(syntax-deprecation #f "_ as an rvalue" ""))
(syntax-deprecation "_ as an rvalue" ""))
e)
(else
(let ((last *in-expand*))
Expand Down Expand Up @@ -202,18 +202,6 @@
(jl-parse-all (open-input-file filename) filename)
(lambda (e) #f)))

(define *depwarn* #t)
(define (jl-parser-depwarn w)
(let ((prev *depwarn*))
(set! *depwarn* (eq? w #t))
prev))

(define *deperror* #f)
(define (jl-parser-deperror e)
(let ((prev *deperror*))
(set! *deperror* (eq? e #t))
prev))

; expand a piece of raw surface syntax to an executable thunk
(define (jl-expand-to-thunk expr)
(parser-wrap (lambda ()
Expand All @@ -229,3 +217,36 @@
(newline)
(prn e))
(lambda () (profile s))))


; --- logging ---
; Utilities for logging messages from the frontend, in a way which can be
; controlled from julia code.

; Log a syntax deprecation from an unknown location
(define (syntax-deprecation what instead)
(syntax-deprecation- what instead 'none 0 #f))

(define (syntax-deprecation- what instead file line exactloc)
(frontend-depwarn (format-syntax-deprecation what instead file line exactloc)
file line))

(define (format-syntax-deprecation what instead file line exactloc)
(string "Deprecated syntax \"" what "\""
(if (or (= line 0) (eq? file 'none))
""
(string (if exactloc " at " " around ") file ":" line))
"."
(if (equal? instead "") ""
(string #\newline "Use \"" instead "\" instead."))))

; Emit deprecation warning via julia logging layer if posible. If not - eg,
; in bootstrap or in the special case that deprecations have been set to
; errors, do the job here.
(define (frontend-depwarn msg file line)
(let ((warnstatus (julia-depwarn msg file line)))
(cond ((eq? warnstatus 2) (error msg))
((eq? warnstatus 1) (io.write *stderr* msg))
; (eq? warnstatus 0) - successfully logged
)))

Loading

0 comments on commit ba866dc

Please sign in to comment.