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

old print syntax throws #2339

Closed
rebcabin opened this issue Sep 26, 2023 · 5 comments · Fixed by #2511
Closed

old print syntax throws #2339

rebcabin opened this issue Sep 26, 2023 · 5 comments · Fixed by #2511
Labels
bug Something isn't working good first issue Good for newcomers Parser Issues or improvements related to parser PyCon India 2023 Devsprint

Comments

@rebcabin
Copy link
Contributor

rebcabin commented Sep 26, 2023

My fingers stupidly typed print syntax from Python 2.17. LPython responded with a barf, but, perhaps a friendly error message would be more appropriate.

repro:

def main():
    print "done!"


if __name__ == '__main__':
    main()

result:

(base) ┌─(~/CLionProjects/lpython/lasr/LP-pycharm)───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────(brian@MacBook-Pro:s003)─┐
└─(13:47:03 on lasr ✭)──> lpython -I. ISSUES/IssueStringMain.py                                                                                                          1 ↵ ──(Tue,Sep26)─┘
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  File "/Users/brian/CLionProjects/lpython/src/bin/lpython.cpp", line 1888
    err = compile_python_to_object_file(arg_file, tmp_o, runtime_library_dir,
  File "/Users/brian/CLionProjects/lpython/src/bin/lpython.cpp", line 786
    LCompilers::Result<LCompilers::LPython::AST::ast_t*> r = parse_python_file(
  File "/Users/brian/CLionProjects/lpython/src/lpython/parser/parser.cpp", line 126
    Result<LPython::AST::Module_t*> res = parse(al, input, prev_loc, diagnostics);
  File "/Users/brian/CLionProjects/lpython/src/lpython/parser/parser.cpp", line 22
    p.parse(s, prev_loc);
  File "/Users/brian/CLionProjects/lpython/src/lpython/parser/parser.cpp", line 53
    m_tokenizer.set_string(inp, prev_loc);
  File "parser.yy", line 1107
  File "/Users/brian/CLionProjects/lpython/src/lpython/parser/semantics.h", line 879
    throw LCompilers::LCompilersException("The string is not recognized by the parser.");
LCompilersException: The string is not recognized by the parser.
@rebcabin rebcabin reopened this Sep 26, 2023
@rebcabin rebcabin changed the title __name__ == '__main__' idiom throws old print syntax throws Sep 26, 2023
@certik
Copy link
Contributor

certik commented Sep 26, 2023

Yes, the error message should be a nice syntax error message as usual. That's a bug in our parser. Thanks!

@certik certik added bug Something isn't working Parser Issues or improvements related to parser good first issue Good for newcomers labels Sep 26, 2023
@Shaikh-Ubaid
Copy link
Collaborator

I think with the current design, we can catch errors for Python 2 print of the forms

print "some string here"
print "string", <sequence of expression here>

To handle other case (like below) where the first argument is not a string, I think we might need to add new rule(s) in the parser.

print xyz
print 523, "string"
print a + b

@Thirumalai-Shaktivel
Copy link
Collaborator

Thirumalai-Shaktivel commented Oct 1, 2023

I think it would be easy to recognize this in the tokenizer instead of the parser. Even if we try to recognize this in the parser, we might face conflict issues.

@Shaikh-Ubaid
Copy link
Collaborator

we might face conflict issues.

Yes, exactly. There can be conflicts between two grammar rules, one for handling the prefix strings and the other for the python2 print.

I wonder if we should handle the prefix strings at the tokenizer level as well. For example, we have this PR #1885 ready. (It might need a rebase/conflict resolution (if any)). Maybe we can then handle the python2 print at the parser level.

@certik What do you say?

@certik
Copy link
Contributor

certik commented Oct 1, 2023

I would say any solution that doesn't complicate or slow down neither the parser nor the tokenizer is ok. So you can try to implement it, and if it is clean, we can merge it. If it can't be done in a clean way, then we'll not do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers Parser Issues or improvements related to parser PyCon India 2023 Devsprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants