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

Correction for short function syntax, closes #356 #359

Merged

Conversation

filipsajdak
Copy link
Contributor

The current implementation does not work for the following code:

main: () = {
    :() = 1;
    [[assert: 1]]
}

It fails with the error:

error: subscript expression [ ] must not be empty (if you were trying to name a C-style array type, use 'std::array' instead) (at '[')

This change introduces a minor correction that moves back parsing to a semicolon (to simulate a double semicolon) for short syntax.

It is not done in the following cases:

:() = 1;(); // immediately called lambda
f(a,b,:() = 1;); // last argument in function call
f(a,:() = 1;,c); // first or in the middle argument

After this change, the original issue is solved.
All regression tests pass. Closes #356

@JohelEGP
Copy link
Contributor

I can confirm that the original issue is solved with this. Thank you.

@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 12, 2023

Let me add that I'm not sure of the implications of this solution. I hadn't thought of IIFE for simple function expressions.

I tested the following on branch main. The handling of IIFEs is inconsistent. Is that because it is a semantic exception that applies to a function scope? In particular, for main and the type, they are all in a statement (as opposed to the declaration-seq of the top-level). See https://godbolt.org/z/vYsqTe3Gq.

Cpp2 code demonstrating the issue.
a0: std::any = :() -> _ = { return 0; }();

// // error: unexpected end of source file
// b0: std::any = :() -> _ = { return 0; }(
// );

// // error: unexpected end of source file
// c0: std::any = :() -> _ = { return 0; }
// ();

d0: std::any = :() -> _ = 0;();

// // error: unexpected end of source file
// e0: std::any = :() -> _ = 0;(
// );

f0: std::any = :() -> _ = 0;
(); // mixed2.cpp2:18:2: error: expected unqualified-id

main: () = {
  a1: std::any = :() -> _ = { return 0; }();

  b1: std::any = :() -> _ = { return 0; }(
  );

  c1: std::any = :() -> _ = { return 0; }
  ();

  d1: std::any = :() -> _ = 0;();

  e1: std::any = :() -> _ = 0;(
  );

  f1: std::any = :() -> _ = 0;
  ();
}

x: type = {
  // Enabled individually, all generate:
  // ```C++
  //   private: std::any 𝘓2 {[]() -> auto;
  // ()}; public: x() = default; x(x const&) = delete; auto operator=(x const&) -> void = delete;
  // ```

  // a2: std::any = :() -> _ = { return 0; }();

  // b2: std::any = :() -> _ = { return 0; }(
  // );

  // c2: std::any = :() -> _ = { return 0; }
  // ();

  // d2: std::any = :() -> _ = 0;();

  // e2: std::any = :() -> _ = 0;(
  // );

  // f2: std::any = :() -> _ = 0;
  // ();
}

Should I open an issue for this?

@filipsajdak
Copy link
Contributor Author

No need for a new issue as this is not yet merged.

@filipsajdak
Copy link
Contributor Author

For the following code:

c0: std::any = :() -> _ = { return 0; }
();

first line is cpp2, the second cpp1:

+/* 2 */ c0: std::any = :() -> _ = { return 0; }
+/* 1 */ ();

Nevertheless it should just inform that there is an issue with missing semicolon.

@filipsajdak
Copy link
Contributor Author

I have pushed the fix - the issue is with the error() method used to report errors.
The method uses curr() internally, which throws when cppfront has processed all tokens.

m += std::string(" (at '") + curr().to_string(true) + "')";

cppfront/source/parse.h

Lines 2787 to 2795 in f83ca9b

auto curr() const
-> token const&
{
if (done()) {
throw std::runtime_error("unexpected end of source file");
}
return (*tokens_)[pos];
}

@filipsajdak
Copy link
Contributor Author

Now the error looks like the following:

bug_352_error_message_semicolon.cpp2...
bug_352_error_message_semicolon.cpp2(1,39): error: ill-formed initializer (after '}')
bug_352_error_message_semicolon.cpp2(1,1): error: unexpected text at end of Cpp2 code section (at 'c0')
bug_352_error_message_semicolon.cpp2(1,0): error: parse failed for section starting here

@JohelEGP
Copy link
Contributor

Yes, with the build from 2b730e8.

Current implementation does not work for the following code:
```cpp
main: () = {
    :() = 1;
    [[assert: 1]]
}
```
It fails with error:
```
error: subscript expression [ ] must not be empty (if you were trying to name a C-style array type, use 'std::array' instead) (at '[')
```

This change introduce small correction that moves back parsing
to semicolon (to simulate double semicolon) for short syntax.

It is not done in the following cases:
```cpp
:() = 1;(); // imediatelly called lambda
f(a,b,:() = 1;); // last argument in function call
f(a,:() = 1;,c); // first or in the middle argument
```

After this change the original issue is solved.
All regression tests pass. Closes hsutter#356
@filipsajdak filipsajdak force-pushed the fsajdak-fix-short-syntax-lambda-and-contract branch from 4b3283f to d737f9e Compare April 16, 2023 00:00
@hsutter hsutter merged commit 19132f1 into hsutter:main Apr 16, 2023
@hsutter
Copy link
Owner

hsutter commented Apr 16, 2023

Looks good, thanks!

@filipsajdak filipsajdak deleted the fsajdak-fix-short-syntax-lambda-and-contract branch April 16, 2023 00:15
zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023
Current implementation does not work for the following code:
```cpp
main: () = {
    :() = 1;
    [[assert: 1]]
}
```
It fails with error:
```
error: subscript expression [ ] must not be empty (if you were trying to name a C-style array type, use 'std::array' instead) (at '[')
```

This change introduce small correction that moves back parsing
to semicolon (to simulate double semicolon) for short syntax.

It is not done in the following cases:
```cpp
:() = 1;(); // imediatelly called lambda
f(a,b,:() = 1;); // last argument in function call
f(a,:() = 1;,c); // first or in the middle argument
```

After this change the original issue is solved.
All regression tests pass. Closes hsutter#356
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.

[BUG] Contract after simple expression function fails
3 participants