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

[BUG] Postfix operator to parenthesized expression isn't parsed in a statement #404

Closed
JohelEGP opened this issue May 1, 2023 · 7 comments · Fixed by #441
Closed

[BUG] Postfix operator to parenthesized expression isn't parsed in a statement #404

JohelEGP opened this issue May 1, 2023 · 7 comments · Fixed by #441
Labels
bug Something isn't working

Comments

@JohelEGP
Copy link
Contributor

JohelEGP commented May 1, 2023

Title: Postfix operator to parenthesized expression isn't parsed in a statement.

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

main: (args) = { (args)&; }

Commands:

cppfront -clean-cpp1 main.cpp2

Expected result:
Same as (https://cpp2.godbolt.org/z/x8qov5xvE):

main: (args) = (args)&;

Actual result and error:

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

JohelEGP commented May 3, 2023

This doesn't work, either: main: () = { (0); } (https://cpp2.godbolt.org/z/MEM756cG9).
It's worth noting that, unparenthesized, it results in (https://cpp2.godbolt.org/z/W9Eeb78dd):

main.cpp2(1,14): error: unused literal or identifier (at '0')

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 6, 2023

This also fails: https://cpp2.godbolt.org/z/qnfW3PPz7.

main: () = {
  x := 0;
  (+x);
}

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 8, 2023

  (:() = 0) as t;
  (:() = 0) is t;

also fail.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 8, 2023

I think parameterized statement may be at fault.
I remember trying out a few of these before that, and having no problems.

@filipsajdak
Copy link
Contributor

But this works:

    :() = 0; as int;
    :() = 0; is int;

@filipsajdak
Copy link
Contributor

@JohelEGP #441 fixes most of this issue.

The only case that is not fixed by this PR is the following:

main: (args) = { (args)&; }

The parser is confused by (args) as it looks like a parameter declaration. The following code works:

main: (args) = { (args&); }

We can solve that in two ways:

  1. Provide diagnostics for code that looks like a parameter list, but in fact, it is not,
  2. Providing backtrack and after successfully parsing parameters but later failing to parse the rest of the statement, we can repeat the parsing without parsing the parameter list,
diff --git a/source/parse.h b/source/parse.h
index 8e6369d..a5cc1a8 100644
--- a/source/parse.h
+++ b/source/parse.h
@@ -5114,6 +5114,9 @@ private:
 
         auto n = std::make_unique<statement_node>();
 
+        //  Remember current position, in case this isn't a valid statement
+        auto start_pos = pos;
+
         //  If a parameter list is allowed here, try to parse one
         if (parameters_allowed) {
             n->parameters = parameter_declaration_list(false, true, false, true);
@@ -5193,6 +5196,10 @@ private:
         }
 
         else {
+            if (n->parameters) {
+                pos = start_pos; // backtrack
+                return statement(semicolon_required, equal_sign, false); // try parsing without parsing parameter list
+            }
             return {};
         }
     }

My gut feeling is that option 1 is the right one - the parsing should be context-free option 2 is a fix for case when it is not context-free.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented May 9, 2023

@JohelEGP #441 fixes most of this issue.

As I suspected, it was parameterized statements.
There were too many errors when starting a statement with parentheses.

We can solve that in two ways:

1. Provide diagnostics for code that looks like a parameter list, but in fact, it is not,

2. Providing backtrack and after successfully parsing parameters but later failing to parse the rest of the statement, we can repeat the parsing without parsing the parameter list,

My gut feeling is that option 1 is the right one - the parsing should be context-free option 2 is a fix for case when it is not context-free.

I do wonder if there's a third way.
Similar to #387 (comment),
a parenthesized identifier can't be the parameter declaration list of a parameterized statement.
That's because the parameterized statement requires its parameters to be initialized
(except the single parameter in a for loop's body).
So when a statement starts with parentheses, either

  • parameters with initializers are parsed before a statement, or
  • no parameter is parsed, they're all expressions.

Anything in-between would be a semantic violation, e.g.:

  • (x:=0, y);. Parameter declarations, but y is uninitialized.
  • (x:=0, y:=0);. Unexpected semicolon.
  • (x) { }. Expected semicolon after expression, before block.
  • (x); { }. OK, expression-statement, then block.

I think the third way I describe is context-free
because you can determine from just parsing that it's invalid.

Cpp2 strictly avoids this, and never requires sema to guide parsing. When I say "context-free parsing" this is the primary thing I have in mind... the compiler (and the human) can always parse the current piece of code without getting non-local information from elsewhere.
-- Extract from https://github.com/hsutter/cppfront/wiki/Design-note%3A-Unambiguous-parsing.

The way I parse a statement that starts with parentheses
is expecting either all expressions or all parameters,
solving any ambiguities by assuming an expression if it can be (so := is required for parameters)
But I may be wrong that such a process counts as context-free.

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