language icon indicating copy to clipboard operation
language copied to clipboard

Can mixin classes be augmented, and to what degree?

Open jakemac53 opened this issue 6 months ago • 3 comments

@munificent @lrhn @eernstg I could use your input on this one :)

Mixin classes are the ones of the form class A = B with C implements D; (see section 12.1 of the language spec).

If I understand correctly (please correct me if I am wrong), these classes have the unique property of not actually introducing a new class A, instead they introduce a type A bound to the mixin application class (B+C above), and it also will implement any interfaces listed. Another unique property of theirs is that constructors are inherited from the superclass.

Question 1: Augmenting with mixins/interfaces

This seems like it would certainly be possible to allow, although we would have to decide upon the syntax. Should the augmentation also be structured like a mixin class? Probably.

Another question is the usefulness of allowing this, I am hard pressed to come up with any compelling use case.

Question 2: Augmenting with instance members

We could say any augmenting members augment the mixin application class. If these are unique classes for each mixin class (not clear to me), then it should be safe to do? This might just be philosophically not desirable though, even if we could specify it in a way that makes sense.

If we do not allow it, this may have some downstream affects on the macro APIs, we will need a different representation for mixin classes (possibly needed anyways), and macros targeting classes probably won't be runnable on them generally. They will have totally bespoke handling in the APIs for execution and introspection.

Special note: Would you be allowed to augment the inherited constructor?

Question 3: Augmenting with static members

I am not sure if this is any different than instance members, but could be.

jakemac53 avatar Jan 05 '24 17:01 jakemac53

Currently, the only case where code-generators use mixin class that I am aware of is Mobx. They use it to enable overriding user-defined getters/methods.

But with macros, they could just augment those instead of creating a subclass. So it's likely to stop using mixin classes.

rrousselGit avatar Jan 05 '24 17:01 rrousselGit

I usually call these declarations "mixin application class" declarations. Using just "mixin class" gets confusing now we also have mixin class declarations.

not actually introducing a new class A

That is a new class. It's the class created by applying the mixins to the superclass and also implementing any mentioned interfaces. Every mixin application creates a new class, and inherits constructors from the superclass (or rather, implicitly gets forwarding constructors where there is no name clash preventing it), which is why a normal class declaration with a mixin can call a superclass constructor at all. In this case, we take that mixin application class, possibly add further interfaces to it, and then we call it A.

All other mixin applications are anonymous, so this one stands out by having a real name.

Ad 1: I see no problem with augmenting such a class just like any other class. The augmentation is added on top of the class, like a thin layer (not a new subclass like a mixin application). If that can augment both a declared method and an inherited method in other classes, it should be able to do something similar here. It's just a class. Implements might have to be careful, depending on how they implemented it.

Ad 2: Mixin application classes are currently unique per mixin application, but we may want to allow implementation freedom about that. The class of mixin application class declarations may have to be unique.ø in all cases. They are the classes of different class declarations. If they add interfaces, they are definitely unique.

So assume at least those classes are unique.

No problem with augmenting the forwarding constructor.

(Can you augment an implicit default constructor? Probably depends on whether a class gets an implicit default constructor before applying augmentations, or after.)

Another option is to not add forwarding constructors until after augmentation, and only do so if no constructor has been added, or no generative constructor. Then you can't augment them, but you can add alternatives. (But that means adding one constructor might inhibit n forwarding constructors.)

Ad 3. I don't think augmenting with static members should be a problem. There is a class declaration, we can access constructors from its name, so why not static methods.

We may need to formalize the underlying model more precisely than we have needed before, to be sure that the model supports everything we do here. But I think it should be possible.

lrhn avatar Jan 05 '24 20:01 lrhn

Thanks, I think mostly I am now only left with the question of whether or not this would actually be useful. Or if we should instead just say they cannot be augmented at all, and keep things simpler. But if we want to, it sounds feasible to allow it.

I usually call these declarations "mixin application class" declarations. Using just "mixin class" gets confusing now we also have mixin class declarations.

I agree it is confusing, I had to search the spec for what to call it, but that says "Mixin Class", should we update it?

That is a new class. It's the class created by applying the mixins to the superclass and also implementing any mentioned interfaces.

Ah yes on a re-read of the spec I think I get it now. It isn't an "alias" to the mixin application class like I was thinking of it, it is truly the mixin application class itself. Makes sense.

(or rather, implicitly gets forwarding constructors where there is no name clash preventing it)

Ah, if it is just a forwarding constructor then I think that definitely helps answer some other questions.

It's just a class. Implements might have to be careful, depending on how they implemented it.

Yeah I do worry there could be some optimizations that have been made based on assumptions that are not necessarily valid any more if we allow augmenting them.

(Can you augment an implicit default constructor? Probably depends on whether a class gets an implicit default constructor before applying augmentations, or after.)

I am quite sure it only gets added after all augmentations, but will check. If we got smarter implicit default constructors, this might be more interesting to support though.

Another option is to not add forwarding constructors until after augmentation, and only do so if no constructor has been added, or no generative constructor. Then you can't augment them, but you can add alternatives.

I think we want to keep them as close to existing behavior as possible. If all the sudden the forwarding constructors are not created, I think that would be surprising. I think the automatic forwarding constructors are the most compelling reason to use this feature at all.

jakemac53 avatar Jan 05 '24 22:01 jakemac53

I am 100% OK with not allowing you to augment mixin application classes. Mixin applications are, as far as I can tell, basically a useless feature as far as solving real-world user problems. I'd honestly be happy removing them from the language entirely.

I suspect that if we do support augmenting them, then it will end up opening up a bunch of random edge case issues and I'd rather not rathole on those for a feature that just isn't that useful in the first place.

munificent avatar Mar 19 '24 19:03 munificent

Lets go ahead and close this, and not allow augmenting them for now.

Not sure if that needs to be made explicit in the spec or not though?

jakemac53 avatar Mar 19 '24 19:03 jakemac53

Apparently, I forgot to respond to this one. Let me just add a couple of comments. ;-)

@jakemac53 wrote:

Not sure if that needs to be made explicit in the spec or not though?

The augmentation feature specification could eliminate 'augment'? as the first part of a mixin application class declaration (that's the second alternative of the classDeclaration rule).

However, we might want to keep it even if basically no augmentation can be performed: We might at least allow an augmentation to add metadata.

By the way, a mixin application class declaration like class B = A with M implements I; is specified to denote a regular class declaration (any mixin application does this, named or not), and that class declaration arguably ought to provide the same quality of service as any other class declaration. It shouldn't matter which syntax is used to declare any given entity, the important thing is what the declared entity is, and the expressive power bestowed on that entity would then follow from the semantics, not the syntax.

So, ideally, I'd recommend that we support augmentations of mixin application classes just like other classes. Adding mixins and/or interfaces can be supported just like it is supported for other class declarations, and it should be equally useful. Adding instance members might be even more useful, as mentioned below. Similarly for adding static members.

We might not want to do it right now because there may be some implementation issues (if the representation of various entities in the tools will make this feature difficult to implement). In any case, it can be an augmentation-libraries-later thing.

@munificent wrote:

basically a useless feature

It's a conciseness feature, and we don't usually eliminate those just because we could write the same thing "manually". For example, if A has 10 constructors with complex signatures then we'll have to copy all 10 of them in order to get the same functionality with B2 that we have with B1:

class A {
  .../* 10 constructors with a verbose signature */...
}

mixin M { ... }

class B1 = A with M; // Done!

class B2 extends A with M {
  ... /* 10 verbose forwarding constructor declarations */ ...
}

Note that we'd also have to maintain those 10 verbose forwarding declarations when the declaration of A evolves.

So if we have a class A with a bunch of useful constructors and several abstract method implementations then we can create a bunch of concrete subclasses by adding mixins to implement the abstract members in various ways. This will be straightforward with mixin application class declarations, but considerable more verbose with regular class declarations.

This might even be more important than previously, if the mixin application class can accommodate the addition of extra or overriding instance members and/or the addition of static members (because they can be in an augmentation).

eernstg avatar Mar 20 '24 10:03 eernstg

By the way, a mixin application class declaration like class B = A with M implements I; is specified to denote a regular class declaration

It's denoting a regular class, but not a "regular class declaration", if that means something of the form class Name ... { body }. In particular, it doesn't inherit the mixed-in members, it declares them.

Take:

class A {}
mixin M {
  int get bar => 37;
}
class B = A with M;
augment class B {
  int get foo => 42;
  augment int get bar => augmented + 5;
}

Here we get to augment the mixed-in member bar, because we are augmenting the class where it's declared, it's not inherited as it would be in class B extends A with M {}. And we are adding foo to that class too. Which I guess is perfectly valid and well-defined.

It does compound the issue of implicit constructors, because mixin application classes inherit (forward) all accessible generative superclass constructors.

Take:

class A {
  A() {}
  A.foo() {}
  A.bar() {}
  A.baz() {}
}
mixin M {}
class B = A with M;
augment class B {
  B.create() => this.foo();
}
augment class B {
  int get foo => 42; // Does this prevent `B` from forwarding to `A.foo`. What does that do to `B.create`?
  augment B.bar() : assert(allIsWell), augmented(); // Can we augment constructors and forward to the augmented constructor?
  B.baz(): super.baz(); // Is this calling *past* `A&M.baz`?
}

Not sure this is a harder problem than just what happens with default constructors if an augment declares a constructor. (But I also don't remember what we did for that.)

lrhn avatar Mar 20 '24 10:03 lrhn

If we only allow adding metadata today, that SGTM.

jakemac53 avatar Mar 20 '24 15:03 jakemac53