Skip to content

Commit

Permalink
Send all parser and lowering depwarns to the logging system (#25257)
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.

Notable changes:

* Depwarn messages go to the logging system, rather than STDERR
* Meta.parse() is given a `depwarn` keyword for convenience in
  controlling deprecation warnings.
* Tests for all parser and lowering deprecation warnings

Detail:

* Ensure that syntax-deprecation, deprecation-message forwards to the logging
  system.  Split these into distinct functions for depwarns coming from the
  parser vs lowering, as these extract line number information in a different
  way.
* Include file and line number as metadata everywhere that the frontend knows
  about it.
* Remove jl_parse_depwarn(), replace flisp *depwarn* / *deperror* with
  simplified *depwarn-opt* handled in one place.
* Replace Base.syntax_deprecation_warnings() with Meta.parse(..., depwarn=false)
* Internal C functions jl_log() and jl_logf() for use when communicating log
  messages from C code.  These will need to be augmented with an async
  jl_safe_log() or something similar when printing log messages from the
  runtime internals.

Todo:
* Figure out a decent `id` for the syntax and lowering depwarn messages
  • Loading branch information
c42f authored and JeffBezanson committed Dec 30, 2017
1 parent 4d1d723 commit 2116b4c
Show file tree
Hide file tree
Showing 17 changed files with 384 additions and 188 deletions.
21 changes: 6 additions & 15 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -200,19 +200,7 @@ 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
end

function parse_input_line(s::String; filename::String="none")
function parse_input_line(s::String; filename::String="none", depwarn=true)
# (expr, pos) = Meta.parse(s, 1)
# (ex, pos) = ccall(:jl_parse_string, Any,
# (Ptr{UInt8},Csize_t,Int32,Int32),
Expand All @@ -221,8 +209,11 @@ function parse_input_line(s::String; filename::String="none")
# throw(Meta.ParseError("extra input after end of expression"))
# end
# expr
ex = ccall(:jl_parse_input_line, Any, (Ptr{UInt8}, Csize_t, Ptr{UInt8}, Csize_t),
s, sizeof(s), filename, sizeof(filename))
# For now, assume all parser warnings are depwarns
ex = with_logger(depwarn ? current_logger() : NullLogger()) do
ccall(:jl_parse_input_line, Any, (Ptr{UInt8}, Csize_t, Ptr{UInt8}, Csize_t),
s, sizeof(s), filename, sizeof(filename))
end
if ex isa Symbol && all(equalto('_'), string(ex))
# remove with 0.7 deprecation
Meta.lower(Main, ex) # to get possible warning about using _ as an rvalue
Expand Down
4 changes: 1 addition & 3 deletions base/docs/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,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
Meta.parse(line, raise = false)
end
x = Meta.parse(line, raise = false, depwarn = false)
# Retrieving docs for macros requires us to make a distinction between the text
# `@macroname` and `@macroname()`. These both parse the same, but are used by
# the docsystem to return different results. The first returns all documentation
Expand Down
16 changes: 12 additions & 4 deletions base/logging.jl
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,9 @@ function logmsg_code(_module, file, line, level, message, exs...)
if k == :_id
# id may be overridden if you really want several log
# statements to share the same id (eg, several pertaining to
# the same progress step).
#
# TODO: Refine this - doing it as is, is probably a bad idea
# for consistency, and is hard to make unique between modules.
# the same progress step). In those cases it may be wise to
# manually call log_record_id to get a unique id in the same
# format.
id = esc(v)
elseif k == :_module
_module = esc(v)
Expand Down Expand Up @@ -360,6 +359,15 @@ end
nothing
end

# Log a message. Called from the julia C code; kwargs is in the format
# Any[key1,val1, ...] for simplicity in construction on the C side.
function logmsg_thunk(level, message, _module, group, id, file, line, kwargs)
real_kws = Any[(kwargs[i],kwargs[i+1]) for i in 1:2:length(kwargs)]
@logmsg(convert(LogLevel, level), message,
_module=_module, _id=id, _group=group,
_file=String(file), _line=line, real_kws...)
end

# Global log limiting mechanism for super fast but inflexible global log
# limiting.
const _min_enabled_level = Ref(Debug)
Expand Down
28 changes: 18 additions & 10 deletions base/meta.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Convenience functions for metaprogramming.
"""
module Meta

using ..CoreLogging

export quot,
isexpr,
show_sexpr,
Expand Down Expand Up @@ -93,15 +95,16 @@ struct ParseError <: Exception
end

"""
parse(str, start; greedy=true, raise=true)
parse(str, start; greedy=true, raise=true, depwarn=true)
Parse the expression string and return an expression (which could later be passed to eval
for execution). `start` is the index of the first character to start parsing. If `greedy` is
`true` (default), `parse` will try to consume as much input as it can; otherwise, it will
stop as soon as it has parsed a valid expression. Incomplete but otherwise syntactically
valid expressions will return `Expr(:incomplete, "(error message)")`. If `raise` is `true`
(default), syntax errors other than incomplete expressions will raise an error. If `raise`
is `false`, `parse` will return an expression that will raise an error upon evaluation.
is `false`, `parse` will return an expression that will raise an error upon evaluation. If
`depwarn` is `false`, deprecation warnings will be suppressed.
```jldoctest
julia> Meta.parse("x = 3, y = 5", 7)
Expand All @@ -111,13 +114,17 @@ julia> Meta.parse("x = 3, y = 5", 5)
(:((3, y) = 5), 13)
```
"""
function parse(str::AbstractString, pos::Int; greedy::Bool=true, raise::Bool=true)
function parse(str::AbstractString, pos::Int; greedy::Bool=true, raise::Bool=true,
depwarn::Bool=true)
# pos is one based byte offset.
# returns (expr, end_pos). expr is () in case of parse error.
bstr = String(str)
ex, pos = ccall(:jl_parse_string, Any,
(Ptr{UInt8}, Csize_t, Int32, Int32),
bstr, sizeof(bstr), pos-1, greedy ? 1 : 0)
# For now, assume all parser warnings are depwarns
ex, pos = with_logger(depwarn ? current_logger() : NullLogger()) do
ccall(:jl_parse_string, Any,
(Ptr{UInt8}, Csize_t, Int32, Int32),
bstr, sizeof(bstr), pos-1, greedy ? 1 : 0)
end
if raise && isa(ex,Expr) && ex.head === :error
throw(ParseError(ex.args[1]))
end
Expand All @@ -129,12 +136,13 @@ function parse(str::AbstractString, pos::Int; greedy::Bool=true, raise::Bool=tru
end

"""
parse(str; raise=true)
parse(str; raise=true, depwarn=true)
Parse the expression string greedily, returning a single expression. An error is thrown if
there are additional characters after the first expression. If `raise` is `true` (default),
syntax errors will raise an error; otherwise, `parse` will return an expression that will
raise an error upon evaluation.
raise an error upon evaluation. If `depwarn` is `false`, deprecation warnings will be
suppressed.
```jldoctest
julia> Meta.parse("x = 3")
Expand All @@ -152,8 +160,8 @@ julia> Meta.parse("1.0.2"; raise = false)
:($(Expr(:error, "invalid numeric constant \"1.0.\"")))
```
"""
function parse(str::AbstractString; raise::Bool=true)
ex, pos = parse(str, 1, greedy=true, raise=raise)
function parse(str::AbstractString; raise::Bool=true, depwarn::Bool=true)
ex, pos = parse(str, 1, greedy=true, raise=raise, depwarn=depwarn)
if isa(ex,Expr) && ex.head === :error
return ex
end
Expand Down
8 changes: 2 additions & 6 deletions base/repl/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -657,9 +657,7 @@ end
LineEdit.reset_state(hist::REPLHistoryProvider) = history_reset_state(hist)

function return_callback(s)
ast = Base.syntax_deprecation_warnings(false) do
Base.parse_input_line(String(take!(copy(LineEdit.buffer(s)))))
end
ast = Base.parse_input_line(String(take!(copy(LineEdit.buffer(s)))), depwarn=false)
if !isa(ast, Expr) || (ast.head != :continue && ast.head != :incomplete)
return true
else
Expand Down Expand Up @@ -932,9 +930,7 @@ function setup_interface(
continue
end
end
ast, pos = Base.syntax_deprecation_warnings(false) do
Meta.parse(input, oldpos, raise=false)
end
ast, pos = Meta.parse(input, oldpos, raise=false, depwarn=false)
if (isa(ast, Expr) && (ast.head == :error || ast.head == :continue || ast.head == :incomplete)) ||
(done(input, pos) && !endswith(input, '\n'))
# remaining text is incomplete (an error, or parser ran to the end but didn't stop with a newline):
Expand Down
12 changes: 3 additions & 9 deletions base/repl/REPLCompletions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ function complete_symbol(sym, ffunc)
# Find module
lookup_name, name = rsplit(sym, ".", limit=2)

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

b, found = get_value(ex, context_module)
if found
Expand Down Expand Up @@ -481,9 +479,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
Base.incomplete_tag(Meta.parse(partial, raise=false))
end
inc_tag = Base.incomplete_tag(Meta.parse(partial, raise=false, depwarn=false))

# if completing a key in a Dict
identifier, partial_key, loc = dict_identifier_key(partial,inc_tag)
Expand Down Expand Up @@ -529,9 +525,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
Meta.parse(partial[frange] * ")", raise=false)
end
ex = Meta.parse(partial[frange] * ")", raise=false, depwarn=false)
if isa(ex, Expr) && ex.head==:call
return complete_methods(ex), start(frange):method_name_end, false
end
Expand Down
77 changes: 44 additions & 33 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,16 +169,56 @@ 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_logmsg(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
{
int kwargs_len = (int)nargs - 6;
if (nargs < 6 || kwargs_len % 2 != 0) {
lerror(fl_ctx, fl_ctx->ArgError, "julia-logmsg: bad argument list - expected "
"level (symbol) group (symbol) id file line msg . kwargs");
}
value_t arg_level = args[0];
value_t arg_group = args[1];
value_t arg_id = args[2];
value_t arg_file = args[3];
value_t arg_line = args[4];
value_t arg_msg = args[5];
value_t *arg_kwargs = args + 6;
if (!isfixnum(arg_level) || !issymbol(arg_group) || !issymbol(arg_id) ||
!issymbol(arg_file) || !isfixnum(arg_line) || !fl_isstring(fl_ctx, arg_msg)) {
lerror(fl_ctx, fl_ctx->ArgError,
"julia-logmsg: Unexpected type in argument list");
}

// Abuse scm_to_julia here to convert arguments. This is meant for `Expr`s
// but should be good enough provided we're only passing simple numbers,
// symbols and strings.
jl_value_t *group=NULL, *id=NULL, *file=NULL, *line=NULL, *msg=NULL;
jl_array_t *kwargs=NULL;
JL_GC_PUSH6(&group, &id, &file, &line, &msg, &kwargs);
group = scm_to_julia(fl_ctx, arg_group, NULL);
id = scm_to_julia(fl_ctx, arg_id, NULL);
file = scm_to_julia(fl_ctx, arg_file, NULL);
line = scm_to_julia(fl_ctx, arg_line, NULL);
msg = scm_to_julia(fl_ctx, arg_msg, NULL);
kwargs = jl_alloc_vec_any(kwargs_len);
for (int i = 0; i < kwargs_len; ++i) {
jl_array_ptr_set(kwargs, i, scm_to_julia(fl_ctx, arg_kwargs[i], NULL));
}
jl_log(numval(arg_level), NULL, group, id, file, line, (jl_value_t*)kwargs, msg);
JL_GC_POP();
return fl_ctx->T;
}

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-logmsg", fl_julia_logmsg },
{ 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 @@ -202,12 +242,7 @@ 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_ERROR)
jl_parse_deperror(fl_ctx, 1);
else
jl_parse_depwarn_(fl_ctx, (int)jl_options.depwarn);
set(symbol(fl_ctx, "*depwarn-opt*"), fixnum(jl_options.depwarn));
}

// There should be no GC allocation while holding this lock
Expand Down Expand Up @@ -400,8 +435,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 @@ -834,28 +867,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
56 changes: 43 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 (underscore-symbol? e)
(syntax-deprecation #f "underscores as an rvalue" ""))
(syntax-deprecation "underscores as an rvalue" "" #f))
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,45 @@
(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 general deprecation message at line node location `lno`
(define (deprecation-message msg lno)
(let* ((lf (extract-line-file lno)) (line (car lf)) (file (cadr lf)))
(frontend-depwarn msg file line)))

; Log a syntax deprecation from line node location `lno`
(define (syntax-deprecation what instead lno)
(let* ((lf (extract-line-file lno)) (line (car lf)) (file (cadr lf)))
(deprecation-message (format-syntax-deprecation what instead file line #f) lno)))

; Extract line and file from a line number node, defaulting to (0, none)
; respectively if lno is absent (`#f`) or doesn't contain a file
(define (extract-line-file lno)
(cond ((or (not lno) (null? lno)) '(0 none))
((not (eq? (car lno) 'line)) (error "lno is not a line number node"))
((length= lno 2) `(,(cadr lno) none))
(else (cdr lno))))

(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."))))

; Corresponds to --depwarn 0="no", 1="yes", 2="error"
(define *depwarn-opt* 1)

; Emit deprecation warning via julia logging layer.
(define (frontend-depwarn msg file line)
; (display (string msg "; file = " file "; line = " line #\newline)))
(case *depwarn-opt*
(1 (julia-logmsg 1000 'depwarn (symbol (string file line)) file line msg))
(2 (error msg))))
Loading

0 comments on commit 2116b4c

Please sign in to comment.