Skip to content

Add diagnostics for bad semicolons errors, closes #352 #362

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

Merged

Conversation

filipsajdak
Copy link
Contributor

@filipsajdak filipsajdak commented Apr 12, 2023

The current cppfront (f83ca9) implementation provides unclear error messages - it is hard to find what causes the error.

This change introduces diagnostics for:

f: () = {}; // error: a compound function body shall not end with a semicolon (at ';')
g: () -> int = 42;; // error: a short function syntax shall end with a single semicolon (at ';')

for v* | all next log() do :(e) = {
  foo();
  bar*;
}; // error: for..do loop body shall not end with a semicolon (at ';')

i := 0;
while i* < container.size() next i++ {
  std::cout << container[i] << std::endl;
}; // error: while loop body shall not end with a semicolon (at ';')

l2 := :() -> int = 24;; // error: a short function syntax shall end with a single semicolon (at ';')

; // error: empty statement is not allowed - remove extra semicolon (at ';')

i := 0;; // error: empty statement is not allowed - remove extra semicolon (at ';')

All regression tests pass. Closes #352

@JohelEGP
Copy link
Contributor

Thank you. I can confirm that the usefulness of the diagnostics have increased by pinpointing the offending tokens. The error: bad statement! all seem redundant. #358 (comment), which already pinpointed the offending token, is correctly unaffected.

@filipsajdak filipsajdak force-pushed the fsajdak-add-diagnostics-for-semicolons branch from 9f05d96 to d41fc77 Compare April 12, 2023 00:50
@filipsajdak
Copy link
Contributor Author

I have removed error: bad statement! - it is even better now (no impact on regression tests).

@filipsajdak filipsajdak force-pushed the fsajdak-add-diagnostics-for-semicolons branch 2 times, most recently from ab25cd1 to cacd826 Compare April 16, 2023 00:01
@filipsajdak filipsajdak force-pushed the fsajdak-add-diagnostics-for-semicolons branch from cacd826 to d806563 Compare April 16, 2023 00:49
@filipsajdak filipsajdak requested a review from hsutter April 16, 2023 00:54
@filipsajdak
Copy link
Contributor Author

@hsutter all requested changes applied.

@filipsajdak filipsajdak force-pushed the fsajdak-add-diagnostics-for-semicolons branch from d806563 to 8674999 Compare April 19, 2023 16:35
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 ';')
```
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 ';')
```
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 ';')
```
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 ';')
```
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 ';')
```
When done() is true last correct token and its position is printed.
@filipsajdak filipsajdak force-pushed the fsajdak-add-diagnostics-for-semicolons branch from 8674999 to 6881f5b Compare April 20, 2023 00:17
@@ -4707,6 +4722,11 @@ class parser
)
-> std::unique_ptr<statement_node>
{
if (!done() && curr().type() == lexeme::Semicolon) {
error("empty statement is not allowed - remove extra semicolon");
Copy link
Owner

Choose a reason for hiding this comment

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

I think this works, but just a note: Emitting an error here (not just returning null) could interfere with productions where statement is one alternative but where if it doesn't parse as a statement we would want to try other fallback production(s) instead. I don't think that actually happens today in the grammar though, so this should be okay.

Copy link
Owner

@hsutter hsutter Apr 20, 2023

Choose a reason for hiding this comment

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

Basically, the scenario would be "is there any production where a statement could appear, where if it's not a statement something else that starts with a ; could appear instead" -- I think there isn't today so it should be safe to reject a ; if we're in a position where a statement could appear

@hsutter hsutter merged commit d4c001d into hsutter:main Apr 20, 2023
@filipsajdak filipsajdak deleted the fsajdak-add-diagnostics-for-semicolons branch April 20, 2023 05:27
zaucy pushed a commit to zaucy/cppfront that referenced this pull request 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 this pull request may close these issues.

[QoI] Improve cryptic error message due to accidental semicolon
3 participants