Skip to content

[QoI] Improve cryptic error message due to accidental semicolon #352

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
JohelEGP opened this issue Apr 11, 2023 · 6 comments · Fixed by #362
Closed

[QoI] Improve cryptic error message due to accidental semicolon #352

JohelEGP opened this issue Apr 11, 2023 · 6 comments · Fixed by #362

Comments

@JohelEGP
Copy link
Contributor

The source locations are nowhere near the semicolon. The implementation should know when a semicolon is not needed, so the error message can be more to the point.

Minimal reproducer (https://godbolt.org/z/5Eovbx4Yb):

g: () = { }
f: () = {
  for v* | all next log() do :(e) = {
    foo();
    bar*;
  };
  g();
}

Commands:

cppfront x.cpp2

Expected result: A diagnostic indicating to remove accidental semicolon.

Actual result and error:

x.cpp2(2,9): error: ill-formed initializer (at '{')
x.cpp2(2,1): error: unexpected text at end of Cpp2 code section (at 'f')
x.cpp2(1,0): error: parse failed for section starting here
@JohelEGP
Copy link
Contributor Author

JohelEGP commented Apr 11, 2023

I got curious about how cppfront would handle $ in identifiers: https://godbolt.org/z/9WaejfYas.

int x$x = 0;
main: () = {
  :() = x$x;
}

The same error message, error: unexpected text at end of Cpp2 code section (at 'main'). In addition to the name of the declaration, if possible, the line's tail of the unexpected text could be printed, e.g.

main.cpp2(2,1): error: unexpected text at end of Cpp2 code section for 'main'
main.cpp2(3,11): error: ... at 'x;'

@JohelEGP
Copy link
Contributor Author

With #358 (comment) in mind, I think I know what's going on. Copying from the OP:

g: () = { }
f: () = {
  for v* | all next log() do :(e) = {
    foo();
    bar*;
  };
  g();
}
x.cpp2(2,9): error: ill-formed initializer (at '{')
x.cpp2(2,1): error: unexpected text at end of Cpp2 code section (at 'f')
x.cpp2(1,0): error: parse failed for section starting here
  • f doesn't match any declaration: x.cpp2(2,1): error: unexpected text at end of Cpp2 code section (at 'f').
  • The preceding error is barely closer to the offending ;: x.cpp2(2,9): error: ill-formed initializer (at '{').

When I watched the talk that unveiled cppfront and came to try it out, more than a few times I wished the error messages were more telling. As a C++ programmer, I guessed it wouldn't want to go down the path of template error messages.

Error messages have been improving. But in particular, those with a stack of source locations, like the one above, seemed like they could do better.

Now, I think it should be possible to improve the status-quo. The parser could keep a record of the furthest it got.
For the OP, I think it should generally be possible to additionally diagnose something like this:

x.cpp2(6,4): error: unrecognized statement (at ';')
x.cpp2(3,3): note: previous statement (at 'for').

@filipsajdak
Copy link
Contributor

@JohelEGP I have made some improvements in diagnostics regarding semicolons. I am running tests - it there will be no impact I will push it.

@JohelEGP
Copy link
Contributor Author

Thank you.

The suggestion at #352 (comment) should work for more than semicolons. For example, #352 (comment) is not about the semicolon. The error message of #358 (comment) already does point to the offending token.

@filipsajdak
Copy link
Contributor

I have pushed the PR: #362

@filipsajdak
Copy link
Contributor

After my PR the following code:

int x$x = 0;
main: () = {
  :() = x$x;
}

produces:

bug_352_error_message_semicolon.cpp2...
bug_352_error_message_semicolon.cpp2(3,11): error: bad statement! (at 'x')

hsutter pushed a commit that referenced this issue Apr 20, 2023
* Add diagnostics for semicolon at the end of function

This change introduce diagnostics when there is a semicolon at
the end of the function body:
```cpp
g: () = {};
```
Generates:
```
error: a compound function body shall not end with a semicolon (at ';')
```
And
```cpp
g: () = 42;;
```
Generates:
```
error: a short function syntax shall end with a single semicolon (at ';')
```

* Add diagnostics for semicolon at the end of for loop

This change introduce diagnostics when there is a semicolon at
the end of the for..do loop body:
```cpp
  for v* | all next log() do :(e) = {
    foo();
    bar*;
  };
```
generates:
```
error: for..do loop body shall not end with a semicolon (at ';')
```

* Add diagnostics for semicolon at the end of while body

This change introduce diagnostics when there is a semicolon at
the end of the while loop body:
```cpp
  i := 0;
  while i* < container.size() next i++ {
    std::cout << container[i] << std::endl;
  };
```
generates:
```
error: while loop body shall not end with a semicolon (at ';')
```

* Add diagnostics for double semicolon at short lambda

This change introduce diagnostics when there is a double semicolon at
the end of the short function body for unnamed function:
```cpp
  l2 := :() -> int = 24;;
```
Generates:
```
error: a short function syntax shall end with a single semicolon (at ';')
```

* Add diagnostics for empty statements (extra semicolon)

This change introduce diagnostics when there is a semicolon
without statement
```cpp
;
```
Generates:
```
error: empty statement is not allowed - remove extra semicolon (at ';')
```
That also handles cases when there is double semicolon after a proper
statemnt:
```cpp
  i := 0;;
```
Generates:
```
error: empty statement is not allowed - remove extra semicolon (at ';')
```

* Fix error() method that throws when done() is true

When done() is true last correct token and its position is printed.
zaucy pushed a commit to zaucy/cppfront that referenced this issue Dec 5, 2023
…r#362)

* Add diagnostics for semicolon at the end of function

This change introduce diagnostics when there is a semicolon at
the end of the function body:
```cpp
g: () = {};
```
Generates:
```
error: a compound function body shall not end with a semicolon (at ';')
```
And
```cpp
g: () = 42;;
```
Generates:
```
error: a short function syntax shall end with a single semicolon (at ';')
```

* Add diagnostics for semicolon at the end of for loop

This change introduce diagnostics when there is a semicolon at
the end of the for..do loop body:
```cpp
  for v* | all next log() do :(e) = {
    foo();
    bar*;
  };
```
generates:
```
error: for..do loop body shall not end with a semicolon (at ';')
```

* Add diagnostics for semicolon at the end of while body

This change introduce diagnostics when there is a semicolon at
the end of the while loop body:
```cpp
  i := 0;
  while i* < container.size() next i++ {
    std::cout << container[i] << std::endl;
  };
```
generates:
```
error: while loop body shall not end with a semicolon (at ';')
```

* Add diagnostics for double semicolon at short lambda

This change introduce diagnostics when there is a double semicolon at
the end of the short function body for unnamed function:
```cpp
  l2 := :() -> int = 24;;
```
Generates:
```
error: a short function syntax shall end with a single semicolon (at ';')
```

* Add diagnostics for empty statements (extra semicolon)

This change introduce diagnostics when there is a semicolon
without statement
```cpp
;
```
Generates:
```
error: empty statement is not allowed - remove extra semicolon (at ';')
```
That also handles cases when there is double semicolon after a proper
statemnt:
```cpp
  i := 0;;
```
Generates:
```
error: empty statement is not allowed - remove extra semicolon (at ';')
```

* Fix error() method that throws when done() is true

When done() is true last correct token and its position is printed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants