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

fix #28990, improve top-level location info #30641

Merged
merged 1 commit into from
Jan 9, 2019
Merged

Conversation

JeffBezanson
Copy link
Member

@JeffBezanson JeffBezanson commented Jan 8, 2019

This fixes #28990, plus the following issue:
Before:

julia> error()
ERROR: 
Stacktrace:
 [1] error() at ./error.jl:42
 [2] top-level scope at none:0

After:

julia> error()
ERROR: 
Stacktrace:
 [1] error() at ./error.jl:42
 [2] top-level scope at REPL[1]:1

This works by (1) passing an initial filename and line number to lowering (so that locations set in a :toplevel expression can apply to the following expression), and (2) making the REPL use the same parsing function that files (and include_string) use, which puts a line node before each expression.

Base.parse_input_line now wraps everything in a :toplevel expression including a line node.

The low-level parser entry points are simplified and renamed to parse-one, parse-all, and parse-file.

The ex === () check in parse I believe was dead code; it looks like something that was left out when we replaced () with nothing a long time ago. All parsing functions return nothing for parse(""), so giving an error here would be breaking but we can consider whether we want to switch to that.

Question 1: I believe the code in interpreter.c setting jl_lineno (which this PR removes) is not necessary, since we should now be able to get line numbers from the interpreter stack trace mechanism. Can anyone think of a reason we need that code?

Question 2: I don't understand the need for the change to repl_filename. Without it, the filename for the first repl input is REPL[0] if the input is very simple, e.g. error() as in the example above. But if I insert even just a single space (within the same input cell) before error(), then you get REPL[1].

@JeffBezanson JeffBezanson added REPL Julia's REPL (Read Eval Print Loop) error handling Handling of exceptions by Julia or the user bugfix This change fixes an existing bug compiler:interpreter labels Jan 8, 2019
@vtjnash
Copy link
Member

vtjnash commented Jan 8, 2019

Can anyone think of a reason we need that code?

I find it can be invaluable for debugging segfaults, when the stack can be all smashed up and such. It's usually the first thing I type when opening gdb. Doesn't need to be particularly reliable (try/finally) or even exported for that use-case though.

end
return ex
end

function parse_input_line(s::String; filename::String="none", depwarn=true)
# For now, assume all parser warnings are depwarns
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the parser has any deprecations, so this support code is dead and should be removed

Copy link
Member Author

Choose a reason for hiding this comment

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

We might add more parser deprecations in the future.

@JeffBezanson JeffBezanson force-pushed the jb/fix28990 branch 2 times, most recently from 1e7333a to f45b559 Compare January 8, 2019 16:34
This gives better location info inside `:toplevel` expressions and
for REPL inputs that aren't block constructs.
The low-level parser entry points are simplified and renamed to parse-one,
parse-all, and parse-file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug compiler:interpreter error handling Handling of exceptions by Julia or the user REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LineNumber nodes in :toplevel expressions are ignored
2 participants