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] Bad operator= error message for @value types #1071

Open
dutkalex opened this issue May 15, 2024 · 4 comments
Open

[BUG] Bad operator= error message for @value types #1071

dutkalex opened this issue May 15, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@dutkalex
Copy link
Contributor

dutkalex commented May 15, 2024

Hi folks!
I ran into a very confusing bug while experimenting with cppfront. Consider the following type:

dummy: @value type = {
    name_: std::string;
    operator=: (out this, in name: std::string) = { name_ = name; }
}

Cppfront refuses to compile this code snippet and produces the following error message:

reproducer.cpp2: error: in operator=, expected 'name_ = ...' initialization statement (because type scope 
object 'name_' does not have a default initializer)
reproducer.cpp2(2,5): error: see declaration for 'name_' here
reproducer.cpp2: error: an operator= body must start with a series of 'member = value;' initialization statements 
for each of the type-scope objects in the same order they are declared, or the member must have a default 
initializer (in type 'dummy')

However, there is nothing wrong with the operator= here. The problem is actually that a value type must be default constructible, which is not the case in this example. This error message got me very confused at first, and I had to look into the sources to understand what was going on. Maybe the missing piece to get a better diagnostic is to require all data members of @value types to be declared with a default initializer. What do you think of this?
I'd be happy to submit a PR to fix this in the definition of the @value metaclass, but I'd rather make sure that my analysis is correct first.

Best regards,
Alex

@dutkalex dutkalex added the bug Something isn't working label May 15, 2024
@bluetarpmedia
Copy link
Contributor

I'm definitely in favour of clear, friendly error messages. But instead of requiring all data members to have default initializers, I'd prefer the error to be something like: A value type must be default constructible. Then the user can choose to either add a default constructor or to add data member initializers.

@dutkalex
Copy link
Contributor Author

dutkalex commented Aug 24, 2024

Hi there!
I think I know how to resolve this problem, but I'm struggling with the actual implementation.

Basically, the root cause here is that the @basic_value metafunction adds a default constructor operator=: (out this) = { } if none is provided, even if such a constructor is not valid.

AFAIU, in order to tell if adding such a constructor is valid, one just has to iterate over all the member variables and check that either:

  • a default initializer is provided, or
  • if the type is defined in cpp2, it is default constructible, or
  • if the type is defined in cpp1, defer somehow the check to the cpp1 compiler.

In terms of code, I don't know how to implement some parts of these checks:

basic_value: (inout t: meta::type_declaration) =
{
    t.copyable();

    for t.get_member_functions() do (inout mf) {
        mf.require( !mf.is_protected() && !mf.is_virtual(),
                    "a value type may not have a protected or virtual function" );
        mf.require( !mf.is_destructor() || mf.is_public() || mf.is_default_access(),
                    "a value type may not have a non-public destructor" );
    }

    if !t.has_default_ctor() {
        for t.get_member_objects() do (in mo) {
            mo.require( 
                mo.has_initializer() || /* if cpp2 type: */ mo./* get the type declaration */.has_default_ctor()
                                        /* else: defer check to cpp1 compiler using std::is_default_constructible */, 
                "a value type must be default constructible" 
            );
        }
        t.add_member( "operator=: (out this) = { }");
    }
}

If some of you have ideas on how to proceed, I'd be very interested to discuss!

@dutkalex dutkalex changed the title [BUG] Bad operator= error message for @value types [BUG] Bad operator= error message for @value types Aug 24, 2024
@dutkalex dutkalex changed the title [BUG] Bad operator= error message for @value types [BUG] Bad operator= error message for @value types Aug 24, 2024
@dutkalex
Copy link
Contributor Author

For reference: #1259

@JohelEGP
Copy link
Contributor

JohelEGP commented Oct 4, 2024

I strongly recommend reading #821.
See also #453.

@basic_value gives you a default constructor if you didn't define one.
See the difference in the error message when you comment out its definition (https://cpp2.godbolt.org/z/crnPzzK9r):

example.cpp2(3,3): error: in operator=, expected 'x = ...' initialization statement (because type scope object 'x' does not have a default initializer)
example.cpp2(2,3): error: see declaration for 'x' here
example.cpp2(3,3): error: an operator= body must start with a series of 'member = value;' initialization statements for each of the type-scope objects in the same order they are declared, or the member must have a default initializer (in type 'u')
example.cpp2: error: in operator=, expected 'x = ...' initialization statement (because type scope object 'x' does not have a default initializer)
example.cpp2(2,3): error: see declaration for 'x' here
example.cpp2: error: an operator= body must start with a series of 'member = value;' initialization statements for each of the type-scope objects in the same order they are declared, or the member must have a default initializer (in type 'u')

I suppose this ties back to #844.

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

3 participants