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] Inconsistent Initialization #823

Open
msadeqhe opened this issue Nov 14, 2023 · 10 comments
Open

[BUG] Inconsistent Initialization #823

msadeqhe opened this issue Nov 14, 2023 · 10 comments
Labels
bug Something isn't working

Comments

@msadeqhe
Copy link

msadeqhe commented Nov 14, 2023

Description

The result of initialization is surprising because by changing the placement of =, its result can be changed, and no one expects to have a different result from the same (args...) in similar variable declarations.

To Reproduce

For example:

point: type = {
    value: i32;
    operator=: (out this, inout i: std::initializer_list<i32>) = {
        value = std::accumulate(i.begin(), i.end(), 0);
    }
    operator=: (out this, a: i32, b: i32) = {
        value = a * b;
    }
}

var1:   point = (1, 2); // value is 1 + 2 = 3
var2: = point   (1, 2); // value is 1 * 2 = 2

I'm using Cppfront from Compiler Explorer.

I expect the value of both var1 and var2 to be the same. But their value members have different values, var1.value is 3, var2.value is 2. On the other hand, their initializations are so much similar, therefore the result is surprising.

Additional Context

Available solutions:

  • Make the behavior of initializations (in the example) to be the same. Ban the programmer from writing any type that has overloaded constructors with both std::initializer_list<TYPE> and TYPE, ... signatures, because it's considered bad API design. Also find a way to fix bad API design of Cpp1 libraries (such as std::vector) in Cpp2 side too by extending them with new constructors.
  • Treat std::initializer_list<TYPE> as the type of a list literal, and devote a notation such as [] to create an instance of it in the same way that string types have their own string literals with notation "".
  • Allow the programmer to call the constructor with either () (direct initialization) or [] (list initialization), and make them to fall back to each other in generic code.

Also discussion #637 and issue #193 are related to this bug. Thanks.

@msadeqhe msadeqhe added the bug Something isn't working label Nov 14, 2023
@JohelEGP
Copy link
Contributor

This sounds like a regression.
x: int = 3.2; won't be narrowing anymore (https://cpp2.godbolt.org/z/e1xGr7bPT):

main: () = {
  x: int = 3.2;
}
build/main.cpp:18:10: error: type 'double' cannot be narrowed to 'int' in initializer list [-Wc++11-narrowing]
   18 |   int x {3.2}; 
      |          ^~~
build/main.cpp:18:10: note: insert an explicit cast to silence this issue
   18 |   int x {3.2}; 
      |          ^~~
      |          static_cast<int>( )
build/main.cpp:18:10: warning: implicit conversion from 'double' to 'int' changes value from 3.2 to 3 [-Wliteral-conversion]
   18 |   int x {3.2}; 
      |         ~^~~
build/main.cpp:18:7: warning: unused variable 'x' [-Wunused-variable]
   18 |   int x {3.2}; 
      |       ^
2 warnings and 1 error generated.

@gregmarr
Copy link
Contributor

@JohelEGP Did you put this comment on the wrong issue?

@JohelEGP
Copy link
Contributor

No.
I was just under the impression that some of the listed available solution would change x: int = 3.2;
from being ill-formed due to a narrowing conversion to being well-formed and narrowing.

@gregmarr
Copy link
Contributor

I don't see how any of those things involving lists relate to initializing an int with a double. Which solution would change that behavior?

@JohelEGP
Copy link
Contributor

I suppose it isn't too much of a problem, actually.

  • Allow the programmer to call the constructor with either () (direct initialization) or [] (list initialization), and make them to fall back to each other in generic code.

This one in particular would change today's x: int = (3.2); to silently narrow (https://cpp2.godbolt.org/z/c6he5jcx6).
Also, having [] fall back to () would do the same for x: int = [3.2];.

@gregmarr
Copy link
Contributor

gregmarr commented Nov 14, 2023

Okay, I was looking at just the code as written, not thinking about adding syntax to call a constructor. I would expect passing 3.2 to func(int x) would be a non-silent narrow, and that's really no different than a theoretical int constructor, so I would expect that to still be an error.

Ah, the narrowing error is currently coming from the cpp1 side.

@AbhinavK00
Copy link

I like option 2, don't want fallback semantics and I don't think there would be any problem if cpp2 would just use {} for built-in types and() for everything else.

@msadeqhe
Copy link
Author

This one in particular would change today's x: int = (3.2); to silently narrow (https://cpp2.godbolt.org/z/c6he5jcx6).
Also, having [] fall back to () would do the same for x: int = [3.2];.

Can arguments be checked for narrowing from Cpp2 side before generating Cpp1 code?

@JohelEGP
Copy link
Contributor

Not generally.
The as cast does it for constants.
But instead of a literal 3.2, it could be a variable y of type double.

@AbhinavK00
Copy link

I think I found a bug with initialization

a := std::vector(1, 2);  //all valid
d := int(4.2) //narrowing conversion but works

Now, I don't know if this is intentional or not but I think this should not work.

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

4 participants