Skip to content

[BUG] inout parameterized statement generates wrong Cpp1 #393

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
ntrel opened this issue Apr 24, 2023 · 17 comments
Closed

[BUG] inout parameterized statement generates wrong Cpp1 #393

ntrel opened this issue Apr 24, 2023 · 17 comments
Labels
bug Something isn't working

Comments

@ntrel
Copy link
Contributor

ntrel commented Apr 24, 2023

    (inout i := 0) while i < 4 next i++ {}

cppfront compiles to Cpp1 fine, but Cpp1 declares the i variable wrongly:

$ g++ -Wall -o "test" "test.cpp" -I/home/nick/git/cppfront/include --std=c++2b
test.cpp: In function ‘int main()’:
test.cpp:52:11: error: cannot bind non-const lvalue reference of type ‘int&’ to an rvalue of type ‘int’
   52 | auto& i = 0;
      |           ^
@ntrel ntrel added the bug Something isn't working label Apr 24, 2023
@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 24, 2023

It's not wrong. I think you're expected to give it a valid initializer just as you would an equally declared parameter (via an argument).

@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 24, 2023

Reading #386 (comment), the issue seems to be that #386 (comment)'s examples don't really work:

Note Because this works naturally for flow control statements, those no longer need a special syntax for if/switch/etc. scoped variables. For example:

(x:int = f()) if x>1		// same as C++17: if (int x = f(); x>1)
(x:int = f()) switch x 		// same as C++17: switch (int x = f(); x)
(x:int = f()) for x>1 next --x	// same as K&R C: for (int x = f(); x>1; --x)
(x:int = f()) for range do	// same as C++20: for (int x = f(); _ : range)
(x:int = f(), i:=0) for range do next ++i
                                // same as C++20: { int i=0; for (int x = f(); _ : range) { ++i;
(x:int = f()) while x>1		// not yet allowed for ‘while’ in C++20
(x:int = f()) do { } while x>1	// not yet allowed for ‘do’ in C++20
(x:int = f()) try			// not yet allowed for ‘try’ in C++20
				// note x is available inside the catch too

I, too, expected a parametrized statement of a single parameter to subsume all of Cpp1's if and loops variable declaration syntax.

@JohelEGP
Copy link
Contributor

For this case, you might want to "pass (the initializer) by" copy: https://godbolt.org/z/dPMvP8q1j.

main: () = {
    (copy i := 0) while i < 4 next i++ {}
}

Though that hits:

main.cpp2(2,19): error: (temporary alpha limitation) parameters scoped to a block/statement must be 'in' (the default) or 'inout'

copy isn't one of the parameter-directions argued against at #386 (comment).

@JohelEGP

This comment was marked as outdated.

@JohelEGP

This comment was marked as resolved.

@JohelEGP

This comment was marked as outdated.

@msadeqhe
Copy link

msadeqhe commented Apr 25, 2023

I think this is the grammar of parameterized statement blocks:

It's obvious enough that if (...) is for initialization or for argument passing in parameterized statement blocks, the syntax will be generally like this:

  • In the following list: construct may be if, while, do, for, next or anything else in the future. (inits) is for initialization and (params) is for argument passing in parameterized statement blocks.
  • (inits) { ... }
  • (inits) construct { ... }
  • (inits) construct (params) { ... }
  • construct (params) { ... }
  • In a nutshell, (params) are after construct, and (inits) are at the start of control structures. It is a simple rule.

Internally (inits) part is directly for declaring local variables in nested scope of control structures:

  • They may be inout by default just like how local variables are not const by default.
  • On the other hand, they may be in by default just like how function parameters are in by default.
  • Or their syntax can be changed from being fuction parameters to be variable declarations (ban in or inout).
  • Or ...

But (params) part is a little different, arguments are passed indirectly, therefore they may be in by default as they are now.


Reading #386 (comment), the issue seems to be that #386 (comment)'s examples don't really work:

Note Because this works naturally for flow control statements, those no longer need a special syntax for if/switch/etc. scoped variables. For example:

(x:int = f()) if x>1		// same as C++17: if (int x = f(); x>1)
(x:int = f()) switch x 		// same as C++17: switch (int x = f(); x)
(x:int = f()) for x>1 next --x	// same as K&R C: for (int x = f(); x>1; --x)
(x:int = f()) for range do	// same as C++20: for (int x = f(); _ : range)
(x:int = f(), i:=0) for range do next ++i
                                // same as C++20: { int i=0; for (int x = f(); _ : range) { ++i;
(x:int = f()) while x>1		// not yet allowed for ‘while’ in C++20
(x:int = f()) do { } while x>1	// not yet allowed for ‘do’ in C++20
(x:int = f()) try			// not yet allowed for ‘try’ in C++20
				// note x is available inside the catch too

I, too, expected a parametrized statement of a single parameter to subsume all of Cpp1's if and loops variable declaration syntax.

@JohelEGP, Good point. It seems the design has changed a little bit. Local variables cannot be references (e.g. & or && in Cpp1). But function parameters can be either in, inout, out, copy, move or forward. Both of them can hold pointer types. Therefore (inits) part of parameterized statement blocks are different from initialization part of Cpp1 control structures.

@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 26, 2023

As an optimization of in, I think copy would still declare the variable as const. I don't think there's a parameter-direction to simulate the variable declaration i := 0;.

Reading 8d2f274#diff-43ba71198da8225f205041c699dba495883d7d416c0b58916bf226aae97bafaeR3196-R3198 made me realize that this actually works: https://godbolt.org/z/z7W1q5ohW.

next: (copy x: int) -> int = x++;
main: () -> int = next(0);

So I think the answer is "use the copy parameter-direction"
as suggested above at #393 (comment).
It should be as easy as adding
&& param->direction() != passing_style::copy
and updating the error message at 928186e#diff-43ba71198da8225f205041c699dba495883d7d416c0b58916bf226aae97bafaeR5124-R5127.

@msadeqhe
Copy link

If I understand your suggestion correctly, in this case copy has overhead for most user-defined types.

@msadeqhe
Copy link

msadeqhe commented Apr 27, 2023

Reading 8d2f274#diff-43ba71198da8225f205041c699dba495883d7d416c0b58916bf226aae97bafaeR3196-R3198 made me realize that this actually works: https://godbolt.org/z/z7W1q5ohW.

You're right. I hadn't read that comment from that commit.

So I get your point now. You mean the behaviour of copy parameter type is similar to local variable declaration, and it should be the default parameter type which is a smart answer to solve the issue.

@AbhinavK00
Copy link

Question: 8d2f274 mentions that copy parameters can be const-qualified. May I ask what is the use of const copy parameters as one should use in parameter in that case. Are there any use cases where in would be insufficient?

Another Question: This one is probably for Herb, how does he plan to handle volatile in cpp2, the same way as cpp or some other new way. I think there was a paper by JF Bastein about deprecating volatile, would Herb like to go that direction and implement volatile_load and volatiles_store instead?

@JohelEGP
Copy link
Contributor

JohelEGP commented Apr 27, 2023

It's likely an error to give a coroutine or std::thread's function a reference parameter. See #393 (comment), https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#SScp-coro, https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-value-capture. See also #76 (comment).

@hsutter
Copy link
Owner

hsutter commented Apr 28, 2023

Thanks! Added copy local parameters.

@hsutter
Copy link
Owner

hsutter commented Apr 28, 2023

This one is probably for Herb, how does he plan to handle volatile in cpp2

Actually primarily by a simple rename... as a cpp2::alien_memory<T> type (or pseudotype), but no cv-qualifiers or similar support in the language. The main problem with volatile aren't the semantics (which are deliberately underspecified and fine for talking about "memory that's outside the C++ program that the compiler can't assume it knows anything about" which is an important low-level concept), but that (a) it's wired into the language as a type qualifier which is undesirable and unnecessary, and (b) the current name is confusing and has baggage and so it should be named something that connotes what it's actually for (and I liked "alien" rather than "foreign" because I think the former has a better and stronger connotation).

@AbhinavK00
Copy link

AbhinavK00 commented Apr 28, 2023

That's great, I was hoping for something along those lines and your simple idea is even better than something I had in mind.

Edit: Regearding @JohelEGP's comment, your comment seems to highlight the use of copy parameters which I know are useful but my question was about const copy parameters. I don't see the point behind them, they're just like const T in cpp which we know are pointless. If you actually answered for const copy and I failed to see the point, sorry but please explain.

@hsutter
Copy link
Owner

hsutter commented Apr 28, 2023

@AbhinavK00 Sure: When you use copy you are asking for a new variable. That variable may be const or not. For a function parameter, that matters in the function body (not to the caller). For a statement/block parameter, the same is true; non-const is a scratch variable that is local to the statement/block that the body can modify, const is one that it can't modify.

hsutter added a commit that referenced this issue Apr 28, 2023
The main problem with `volatile` isn't the semantics -- those are are deliberately underspecified, and appropriate for talking about "memory that's outside the C++ program that the compiler can't assume it knows anything about" which is an important low-level concept. The problems with `volatile` are that (a) it's wired into the language as a type qualifier which is undesirable and unnecessary, and (b) the current name is confusing and has baggage and so it should be named something that connotes what it's actually for (and I like "alien" rather than "foreign" because I think "alien" has a better and stronger connotation).
@AbhinavK00
Copy link

Ah ok, I was thinking only for functions and there it doesn't matter to caller so I couldn't see any use but it may make sense for block parameters which I currently have to wrap my head around.

zaucy pushed a commit to zaucy/cppfront that referenced this issue Dec 5, 2023
Allow `copy` statement/block parameter, closes hsutter#393

Allow `class` and `struct` and other reclaimed words as declared names, closes hsutter#394
zaucy pushed a commit to zaucy/cppfront that referenced this issue Dec 5, 2023
The main problem with `volatile` isn't the semantics -- those are are deliberately underspecified, and appropriate for talking about "memory that's outside the C++ program that the compiler can't assume it knows anything about" which is an important low-level concept. The problems with `volatile` are that (a) it's wired into the language as a type qualifier which is undesirable and unnecessary, and (b) the current name is confusing and has baggage and so it should be named something that connotes what it's actually for (and I like "alien" rather than "foreign" because I think "alien" has a better and stronger connotation).
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

No branches or pull requests

5 participants