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] @metaclass prevents type inheriting #453

Open
realgdman opened this issue May 13, 2023 · 11 comments
Open

[BUG] @metaclass prevents type inheriting #453

realgdman opened this issue May 13, 2023 · 11 comments
Labels
bug Something isn't working

Comments

@realgdman
Copy link

Having this: Base prevents type with some @meta from compiling

Reproduce Code

EmptyBase: type = {}
BaseBV: @basic_value type = {}
BV_BV: @basic_value type = { this: BaseBV; } //error
BV_B: @basic_value type = { this: EmptyBase; } //error
I_B: @interface type = { this: EmptyBase; } //error

https://cpp2.godbolt.org/z/Y5sY4a39G

Version
latest (38aec57)

Command lines
cppfront/cppfront $1.cpp2 -p
clang++-15 -Icppfront/include $1.cpp -std=c++20 -o $1

Expected result
Successful compilation or clearer diagnostic if that's intended

Actual result
@basic_value or @value or @struct:
error: expected 'BaseBV = ...' initialization statement - an operator= body must start with a series of 'member = value;'

@interface:
error: while applying @interface - interfaces may not contain data objects

Additional context
This is specifically about what seems as valid combinations, like @interface::@interface

I'm not talking about impossible combinations, like @polymorphic_base::@copyable, that could be another issue.

Which actually compiles, giving cpp1 warning about

@realgdman realgdman added the bug Something isn't working label May 13, 2023
@realgdman
Copy link
Author

realgdman commented May 17, 2023

Regarding @basic_value inheriting @basic_value this might be reasonable error, requiring programmer to write this: BaseBV = ()

Regarding @interface, it seems because m.is_object() is true for this:. This may be right on parse level, but not necessary on semantic/reflection level. Currently it can be workaround with m.require( !m.is_object() || m.name()=="this") inside @interface

@JohelEGP
Copy link
Contributor

JohelEGP commented Aug 23, 2023

This is now #779.

@basic_value or @value or @struct:
error: expected 'BaseBV = ...' initialization statement - an operator= body must start with a series of 'member = value;'

With @basic_value, you opt-into @copyable, which is documented as

// A type with (copy and move) x (construction and assignment)

@copyable also injects operator=: (out this, that) = { }.
But types with this data members only lower a copy constructor from that.
So for types with this data members,
injecting operator=: (out this, that) = { } isn't enough for @copyable to meet its documented behavior.
I think @copyable should reject types with this data members.

By the way,
I've been using @basic_values for polymorphic types,
and this is how I ran into this (https://cpp2.godbolt.org/z/xnoEzc6GE):

sf_vertices: @basic_value type = { }
my_vertices: @basic_value type = {
  this: sf_vertices = ();
  operator=: (out this, size: i32) = _ = size;
}
tiles: type = {
  public vertices: my_vertices;
  operator=: (out this, size: i32) = vertices = size;
  private operator=: (inout this, _: i32) = vertices = my_vertices(0);
}
main: () = { }
main.cpp2:6:9: warning: definition of implicit copy assignment operator for 'my_vertices' is deprecated because it has a user-provided copy constructor [-Wdeprecated-copy-with-user-provided-copy]
    6 | public: my_vertices(my_vertices const& that);
      |         ^
main.cpp2:10:54: note: in implicit copy assignment operator for 'my_vertices' first required here
   10 |                                             vertices = my_vertices(0);
      |                                                      ^
1 warning generated.

@JohelEGP
Copy link
Contributor

I've been using @basic_values for polymorphic types,

Not "polymorphic types", but "implementation inheritance".
My intent was to opt-into the bases' rule of N.

@JohelEGP
Copy link
Contributor

So what I really want is Cpp1's defaults.
Which is equivalent to defaulting all special member functions (default constructor included).
I don't think anything in Cpp2 can give you that for a type that has this data members.

@JohelEGP
Copy link
Contributor

For some more context, I'm writing a shim.
I'm adding a strongly typed interface to a library's types.
I use implementation inheritance because not everything needs wrapping.

I don't think anything in Cpp2 can give you that for a type that has this data members.

Given that @basic_value doesn't work for my use case,
these are my current options AFAIK:

  • Manually write the default special member functions to forward to the base one.
  • Write a CRTP template to opt-into simulating the Cpp1 defaults.

@JohelEGP
Copy link
Contributor

  • Manually write the default special member functions to forward to the base one.

This is actually the only option.
A type with a this data member requires manually writing all operator=s,
because none is generated.
So the only way to opt-into the Cpp1 defaults is by having a metafunction to do just that.

@JohelEGP
Copy link
Contributor

JohelEGP commented Aug 24, 2023

Instead of @basic_value, this is what I have to use, omitting as necessary:

  operator=: (out this) = { }
  operator=: (out this, that)        = { }
  operator=: (out this, move that)   = { }
  operator=: (inout this, that)      = { }
  operator=: (inout this, move that) = { }

The default constructor uses the data members' initializers,
and the others default to the same memberwise operation.

@JohelEGP
Copy link
Contributor

Regarding @interface, it seems because m.is_object() is true for this:. This may be right on parse level, but not necessary on semantic/reflection level. Currently it can be workaround with m.require( !m.is_object() || m.name()=="this") inside @interface

This is the documentation for @interface:

// an abstract base class having only pure virtual functions

There's no way to know, nor assert at compile-time,
that a base type makes (or not) the type being defined meet that documented behavior.

@JohelEGP
Copy link
Contributor

JohelEGP commented Aug 24, 2023

So what I really want is Cpp1's defaults.
Which is equivalent to defaulting all special member functions (default constructor included).

I.e., the rule of zero.

Using @basic_value with a data member of std::unique_ptr
can't be used to mimic the fantastic Cpp1's rule of zero (https://cpp2.godbolt.org/z/zafe6WM16):

t: @basic_value type = {
  p: std::unique_ptr<i32> = ();
  operator=: (out this, x: i32) = p = new<i32>(x);
}
main: () = { }
main.cpp2:6:35: error: call to implicitly-deleted copy constructor of 'std::unique_ptr<cpp2::i32>' (aka 'unique_ptr<int>')
    6 |                                 : p{ that.p }{}
      |                                   ^~~~~~~~~~~
main.cpp2:9:35: error: object of type 'std::unique_ptr<cpp2::i32>' (aka 'unique_ptr<int>') cannot be assigned because its copy assignment operator is implicitly deleted
    9 |                                 p = that.p;
      |                                   ^

Instead of @basic_value, this is what I have to use, omitting as necessary:

  operator=: (out this) = { }
  operator=: (out this, that)        = { }
  operator=: (out this, move that)   = { }
  operator=: (inout this, that)      = { }
  operator=: (inout this, move that) = { }

The default constructor uses the data members' initializers,
and the others default to the same memberwise operation.

This happens to work, but is unfortunately mechanic and verbose (https://cpp2.godbolt.org/z/rWfG5cvTz):

t: type = {
  p: std::unique_ptr<i32> = ();
  operator=: (out this, x: i32) = p = new<i32>(x);
  operator=: (out this) = { }
  // operator=: (out this, that)        = { }
  operator=: (out this, move that)   = { }
  // operator=: (inout this, that)      = { }
  operator=: (inout this, move that) = { }
}
main: () = { }

@JohelEGP
Copy link
Contributor

Fixing #547 as requested would make my uses of @basic_value above work (and my actual use case, too).
But then @copyable's behavior would be different from what it documents.

@JohelEGP
Copy link
Contributor

This happens to work, but is unfortunately mechanic and verbose (https://cpp2.godbolt.org/z/rWfG5cvTz):

And this is how it has to look if it's a template of arity 1 (https://cpp2.godbolt.org/z/rYqonvqYP):

t: <T> type = {
  p: T = ();
  operator=: (out this, x: i32) = p = new<i32>(x);
  operator=: (out this) = { }
  operator=: (out this, that) requires std::copyable<T>   = { }
  operator=: (out this, move that)                        = { }
  operator=: (inout this, that) requires std::copyable<T> = { }
  operator=: (inout this, move that)                      = { }
}
main: () = {
  static_assert(std::movable<t<std::unique_ptr<i32>>>);
  static_assert(!std::copyable<t<std::unique_ptr<i32>>>);
}

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

2 participants