-
Notifications
You must be signed in to change notification settings - Fork 165
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
Print diagnostics in each interactive loop #2809
Conversation
I think this works and does the job: (lp) ubaid@DESKTOP-TEN39M0:~/projects/lpython$ ./src/bin/lpython
Interactive LPython. Experimental prototype, not ready for end users.
LPython version: 0.22.0-53-g1ef4675ae
* Use Ctrl-D to exit
* Use Enter to submit
* Use Alt-Enter or Ctrl-N to make a new line
- Editing (Keys: Left, Right, Home, End, Backspace, Delete)
- History (Keys: Up, Down)
>>> i: i32 = 5 1,11 ]
>>> from math import sin 1,21 ]
>>> sin(i) 1,7 ]
semantic error: `x` argument of `Sin` must be real or complex
--> input:1:5
|
1 | sin(i)
| ^
Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.
>>> pow(2, 3) 1,10 ]
style suggestion: Could have used '**' instead of 'pow'
--> input:1:1
|
1 | pow(2, 3)
| ^^^^^^^^^ '**' could be used instead
Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.
8
>>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for fixing this!
@@ -886,6 +886,8 @@ int interactive_python_repl( | |||
res = fe.evaluate(code_string, verbose, lm, pass_manager, diagnostics); | |||
if (res.ok) { | |||
r = res.result; | |||
std::cerr << diagnostics.render(lm, compiler_options); | |||
diagnostics.clear(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, I think this approach is good to print style info/warnings. But for printing errors (semantic errors like in #2757), we should fix the python_ast_to_asr
to return the result as not ok (that is res.ok = False
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a new issue for this here #2810. I think this PR is good to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this as well. The reason is that for the intrinsic functions, we used append_error
instead of throwing a semantic error. This could potentially be resolved by throwing the error later from python_ast_to_asr
if the intrinsic function returns null.
Fixed #2807
Fixed #2757
In interactive mode, some errors or suggestions were added to the diagnostics but did not throw any errors. These should also be printed.