Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: bring back v0.6 scope rules in the REPL #33864

Merged
merged 3 commits into from
Jan 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ New language features
Language changes
----------------

* The interactive REPL now uses "soft scope" for top-level expressions: an assignment inside a
scope block such as a `for` loop automatically assigns to a global variable if one has been
defined already. This matches the behavior of Julia versions 0.6 and prior, as well as
[IJulia](https://github.com/JuliaLang/IJulia.jl).
Note that this only affects expressions interactively typed or pasted directly into the
default REPL ([#28789], [#33864]).

* Outside of the REPL (e.g. in a file), assigning to a variable within a top-level scope
block is considered ambiguous if a global variable with the same name exists.
A warning is given if that happens, to alert you that the code will work differently
than in the REPL.
A new command line option `--warn-scope` controls this warning ([#33864]).

* Converting arbitrary tuples to `NTuple`, e.g. `convert(NTuple, (1, ""))` now gives an error,
where it used to be incorrectly allowed. This is because `NTuple` refers only to homogeneous
tuples (this meaning has not changed) ([#34272]).
Expand All @@ -19,7 +32,6 @@ Language changes
(in addition to the one that enters the help mode) to see the full
docstring. ([#25930])


Multi-threading changes
-----------------------

Expand Down
1 change: 1 addition & 0 deletions base/options.jl
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ struct JLOptions
outputji::Ptr{UInt8}
output_code_coverage::Ptr{UInt8}
incremental::Int8
warn_scope::Int8
end

# This runs early in the sysimage != is not defined yet
Expand Down
537 changes: 331 additions & 206 deletions doc/src/manual/variables-and-scoping.md

Large diffs are not rendered by default.

60 changes: 54 additions & 6 deletions src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,34 @@ static jl_value_t *scm_to_julia(fl_context_t *fl_ctx, value_t e, jl_module_t *mo
static value_t julia_to_scm(fl_context_t *fl_ctx, jl_value_t *v);
static jl_value_t *jl_expand_macros(jl_value_t *expr, jl_module_t *inmodule, struct macroctx_stack *macroctx, int onelevel);

value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
{
// tells whether a var is defined in and *by* the current module
argcount(fl_ctx, "defined-julia-global", nargs, 1);
(void)tosymbol(fl_ctx, args[0], "defined-julia-global");
jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
jl_sym_t *var = jl_symbol(symbol_name(fl_ctx, args[0]));
jl_binding_t *b = jl_get_module_binding(ctx->module, var);
StefanKarpinski marked this conversation as resolved.
Show resolved Hide resolved
return (b != NULL && b->owner == ctx->module) ? fl_ctx->T : fl_ctx->F;
}

value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
{
jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
assert(ctx->module);
return fixnum(jl_module_next_counter(ctx->module));
}

value_t fl_julia_current_file(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
{
return symbol(fl_ctx, jl_filename);
}

value_t fl_julia_current_line(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
{
return fixnum(jl_lineno);
}

// Check whether v is a scalar for purposes of inlining fused-broadcast
// arguments when lowering; should agree with broadcast.jl on what is a
// scalar. When in doubt, return false, since this is only an optimization.
Expand Down Expand Up @@ -197,9 +218,12 @@ value_t fl_julia_logmsg(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
}

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 },
{ "julia-current-file", fl_julia_current_file },
{ "julia-current-line", fl_julia_current_line },
{ NULL, NULL }
};

Expand Down Expand Up @@ -227,6 +251,7 @@ static void jl_init_ast_ctx(jl_ast_context_t *ast_ctx) JL_NOTSAFEPOINT
ctx->task = NULL;
ctx->module = NULL;
set(symbol(fl_ctx, "*depwarn-opt*"), fixnum(jl_options.depwarn));
set(symbol(fl_ctx, "*scopewarn-opt*"), fixnum(jl_options.warn_scope));
}

// There should be no GC allocation while holding this lock
Expand Down Expand Up @@ -859,9 +884,10 @@ jl_value_t *jl_parse_eval_all(const char *fname,
}
// expand non-final expressions in statement position (value unused)
expression =
fl_applyn(fl_ctx, 3,
symbol_value(symbol(fl_ctx, iscons(cdr_(ast)) ? "jl-expand-to-thunk-stmt" : "jl-expand-to-thunk")),
expression, symbol(fl_ctx, jl_filename), fixnum(jl_lineno));
fl_applyn(fl_ctx, 4,
symbol_value(symbol(fl_ctx, "jl-expand-to-thunk-warn")),
expression, symbol(fl_ctx, jl_filename), fixnum(jl_lineno),
iscons(cdr_(ast)) ? fl_ctx->T : fl_ctx->F);
}
jl_get_ptls_states()->world_age = jl_world_counter;
form = scm_to_julia(fl_ctx, expression, inmodule);
Expand Down Expand Up @@ -1171,6 +1197,13 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand1(jl_value_t *expr, jl_module_t *inmodule
return expr;
}

// Lower an expression tree into Julia's intermediate-representation.
JL_DLLEXPORT jl_value_t *jl_expand(jl_value_t *expr, jl_module_t *inmodule)
{
return jl_expand_with_loc(expr, inmodule, "none", 0);
}

// Lowering, with starting program location specified
JL_DLLEXPORT jl_value_t *jl_expand_with_loc(jl_value_t *expr, jl_module_t *inmodule,
const char *file, int line)
{
Expand All @@ -1183,10 +1216,25 @@ JL_DLLEXPORT jl_value_t *jl_expand_with_loc(jl_value_t *expr, jl_module_t *inmod
return expr;
}

// Lower an expression tree into Julia's intermediate-representation.
JL_DLLEXPORT jl_value_t *jl_expand(jl_value_t *expr, jl_module_t *inmodule)
// Same as the above, but printing warnings when applicable
JL_DLLEXPORT jl_value_t *jl_expand_with_loc_warn(jl_value_t *expr, jl_module_t *inmodule,
const char *file, int line)
{
return jl_expand_with_loc(expr, inmodule, "none", 0);
JL_TIMING(LOWERING);
JL_GC_PUSH1(&expr);
expr = jl_copy_ast(expr);
expr = jl_expand_macros(expr, inmodule, NULL, 0);
jl_ast_context_t *ctx = jl_ast_ctx_enter();
fl_context_t *fl_ctx = &ctx->fl;
JL_AST_PRESERVE_PUSH(ctx, old_roots, inmodule);
value_t arg = julia_to_scm(fl_ctx, expr);
value_t e = fl_applyn(fl_ctx, 4, symbol_value(symbol(fl_ctx, "jl-expand-to-thunk-warn")), arg,
symbol(fl_ctx, file), fixnum(line), fl_ctx->F);
expr = scm_to_julia(fl_ctx, e, inmodule);
JL_AST_PRESERVE_POP(ctx, old_roots);
jl_ast_ctx_leave(ctx);
JL_GC_POP();
return expr;
}

// expand in a context where the expression value is unused
Expand Down
41 changes: 35 additions & 6 deletions src/jlfrontend.scm
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
'(error "malformed expression"))))
thk))

;; this is overwritten when we run in actual julia
(define (defined-julia-global v) #f)
(define (julia-current-file) 'none)
(define (julia-current-line) 0)

;; parser entry points

;; parse one expression (if greedy) or atom, returning end position
Expand Down Expand Up @@ -128,16 +133,38 @@
(begin0 (expand-toplevel-expr-- e file line)
(set! *in-expand* last))))))

; expand a piece of raw surface syntax to an executable thunk
(define (jl-expand-to-thunk expr file line)
;; used to collect warnings during lowering, which are usually discarded
;; unless logging is requested
(define lowering-warning (lambda lst (void)))

;; expand a piece of raw surface syntax to an executable thunk

(define (expand-to-thunk- expr file line)
(error-wrap (lambda ()
(expand-toplevel-expr expr file line))))

(define (expand-to-thunk-stmt- expr file line)
(expand-to-thunk- (if (toplevel-only-expr? expr)
expr
`(block ,expr (null)))
file line))

(define (jl-expand-to-thunk-warn expr file line stmt)
(let ((warnings '()))
(with-bindings
((lowering-warning (lambda lst (set! warnings (cons lst warnings)))))
(begin0
(if stmt
(expand-to-thunk-stmt- expr file line)
(expand-to-thunk- expr file line))
(for-each (lambda (args) (apply julia-logmsg args))
(reverse warnings))))))

(define (jl-expand-to-thunk expr file line)
(expand-to-thunk- expr file line))

(define (jl-expand-to-thunk-stmt expr file line)
(jl-expand-to-thunk (if (toplevel-only-expr? expr)
expr
`(block ,expr (null)))
file line))
(expand-to-thunk-stmt- expr file line))

(define (jl-expand-macroscope expr)
(error-wrap (lambda ()
Expand Down Expand Up @@ -215,6 +242,8 @@
(if (equal? instead "") ""
(string #\newline "Use `" instead "` instead."))))

(define *scopewarn-opt* 1)

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

Expand Down
18 changes: 15 additions & 3 deletions src/jloptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ jl_options_t jl_options = { 0, // quiet
NULL, // output-ji
NULL, // output-code_coverage
0, // incremental
0 // image_file_specified
0, // image_file_specified
JL_OPTIONS_WARN_SCOPE_ON // ambiguous scope warning
};

static const char usage[] = "julia [switches] -- [programfile] [args...]\n";
Expand Down Expand Up @@ -109,7 +110,8 @@ static const char opts[] =

// error and warning options
" --depwarn={yes|no|error} Enable or disable syntax and method deprecation warnings (\"error\" turns warnings into errors)\n"
" --warn-overwrite={yes|no} Enable or disable method overwrite warnings\n\n"
" --warn-overwrite={yes|no} Enable or disable method overwrite warnings\n"
" --warn-scope={yes|no} Enable or disable warning for ambiguous top-level scope\n\n"
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe yes|no|error so people can opt into making this fatal?

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also be added later.


// code generation options
" -C, --cpu-target <target> Limit usage of CPU features up to <target>; set to \"help\" to see the available options\n"
Expand Down Expand Up @@ -169,6 +171,7 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
opt_output_bc,
opt_depwarn,
opt_warn_overwrite,
opt_warn_scope,
opt_inline,
opt_polly,
opt_trace_compile,
Expand Down Expand Up @@ -225,6 +228,7 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
{ "output-incremental",required_argument, 0, opt_incremental },
{ "depwarn", required_argument, 0, opt_depwarn },
{ "warn-overwrite", required_argument, 0, opt_warn_overwrite },
{ "warn-scope", required_argument, 0, opt_warn_scope },
{ "inline", required_argument, 0, opt_inline },
{ "polly", required_argument, 0, opt_polly },
{ "trace-compile", required_argument, 0, opt_trace_compile },
Expand Down Expand Up @@ -554,7 +558,15 @@ JL_DLLEXPORT void jl_parse_opts(int *argcp, char ***argvp)
else if (!strcmp(optarg,"no"))
jl_options.warn_overwrite = JL_OPTIONS_WARN_OVERWRITE_OFF;
else
jl_errorf("julia: invalid argument to --warn-overwrite={yes|no|} (%s)", optarg);
jl_errorf("julia: invalid argument to --warn-overwrite={yes|no} (%s)", optarg);
break;
case opt_warn_scope:
if (!strcmp(optarg,"yes"))
jl_options.warn_scope = JL_OPTIONS_WARN_SCOPE_ON;
else if (!strcmp(optarg,"no"))
jl_options.warn_scope = JL_OPTIONS_WARN_SCOPE_OFF;
else
jl_errorf("julia: invalid argument to --warn-scope={yes|no} (%s)", optarg);
break;
case opt_inline:
if (!strcmp(optarg,"yes"))
Expand Down
Loading