Skip to content

[BUG] Wrong location for ill formed definitions #488

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

Closed
MaxSagebaum opened this issue Jun 2, 2023 · 6 comments · Fixed by #499
Closed

[BUG] Wrong location for ill formed definitions #488

MaxSagebaum opened this issue Jun 2, 2023 · 6 comments · Fixed by #499
Labels
bug Something isn't working

Comments

@MaxSagebaum
Copy link
Contributor

Title: Wrong location for ill formed definitions

Description:

A wrong definition like int a = 4; should be reported with the correct line.

Minimal reproducer (https://cpp2.godbolt.org/z/vcb5xWKxz):

func: () = {
    a : int = 5;
    int b = 4;
    c: int = 3;
}

Commands:

cppfront main.cpp2 -o main.cpp

Expected result:

main.cpp2(3,12): error: ill-formed initializer (at ' int b = 4;')
main.cpp2(1,1): error: unexpected text at end of Cpp2 code section (at 'func')
main.cpp2(1,0): error: parse failed for section starting here

Actual result and error:

main.cpp2(1,12): error: ill-formed initializer (at '{')
main.cpp2(1,1): error: unexpected text at end of Cpp2 code section (at 'func')
main.cpp2(1,0): error: parse failed for section starting here
@MaxSagebaum MaxSagebaum added the bug Something isn't working label Jun 2, 2023
@JohelEGP
Copy link
Contributor

JohelEGP commented Jun 2, 2023

See also #358 (comment), #352 (comment).

@MaxSagebaum
Copy link
Contributor Author

Thanks for the references. I could have a look at this kind of error next week and try to improve the message. Is anybody else working on this? So we do not come into conflict?

@JohelEGP
Copy link
Contributor

JohelEGP commented Jun 2, 2023

My impression is that the logic isn't centralized,
so it'd have to be done on a case-by-case basis.

@JohelEGP
Copy link
Contributor

JohelEGP commented Jun 2, 2023

That means there's little chance of conflict.
For example,
your reproducer fails when parsing the next statement.
#358 (comment) fails at the end of a particular expression production.
#352 (comment) failed at the end of a statement.

@MaxSagebaum
Copy link
Contributor Author

Ok, then will try to hunt it down next week.

@MaxSagebaum
Copy link
Contributor Author

I could track it down yesterday. The stacktrace for the parsing error is:

#0  cpp2::parser::unnamed_declaration (this=0x7fffffffc670, start=..., semicolon_required=true, captures_allowed=false, named=true, is_parameter=false, is_template_parameter=false, id=std::unique_ptr<cpp2::unqualified_id_node> = {...}, access=cpp2::accessibility::default_)
    at ./source/parse.h:5966
#1  0x0000000000431eee in cpp2::parser::declaration (this=0x7fffffffc670, semicolon_required=true, is_parameter=false, is_template_parameter=false) at ./source/parse.h:6684
#2  0x000000000042b17a in cpp2::parser::statement (this=0x7fffffffc670, semicolon_required=true, equal_sign=..., parameters_allowed=true) at ./source/parse.h:5276
#3  0x000000000042b813 in cpp2::parser::compound_statement (this=0x7fffffffc670, equal_sign=..., allow_single_unbraced_statement=false) at ./source/parse.h:5373
#4  0x000000000042b0c8 in cpp2::parser::statement (this=0x7fffffffc670, semicolon_required=true, equal_sign=..., parameters_allowed=false) at ./source/parse.h:5270
#5  0x000000000042fb08 in cpp2::parser::unnamed_declaration (this=0x7fffffffc670, start=..., semicolon_required=true, captures_allowed=false, named=true, is_parameter=false, is_template_parameter=false, id=std::unique_ptr<cpp2::unqualified_id_node> = {...}, access=cpp2::accessibility::default_)
    at ./source/parse.h:6207
#6  0x0000000000431eee in cpp2::parser::declaration (this=0x7fffffffc670, semicolon_required=true, is_parameter=false, is_template_parameter=false) at ./source/parse.h:6684
#7  0x0000000000432357 in cpp2::parser::translation_unit (this=0x7fffffffc670) at ./source/parse.h:6750
#8  0x000000000042203c in cpp2::parser::parse (this=0x7fffffffc670, tokens_=std::vector of length 24, capacity 32 = {...}, generated_tokens_=std::deque with 0 elements) at ./source/parse.h:3385
#9  0x000000000043cf14 in cpp2::cppfront::cppfront (this=0x7ffffffe65e0, filename="src/temp.cpp2") at cppfront.cpp:1157
#10 0x0000000000414a9a in main (argc=2, argv=0x7fffffffcd68) at cppfront.cpp:5926

The best position to insert it was at frame 3. Adding an error at frame 0 would have broken everything. The principle that a creation failure does not imply a parse failure makes it not possible. Adding it at frame 3 did not change anything in the regression tests.

The if prevents an error message when a more detailed message was already added to the stack and also prevents, that the same message is printed twice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants