experimental 'class' doesn't work with existing objects

[jjn1056]

Crossposted from Perl-Apollo/Corinna#105 because its completely unclear how to start a conversation about where core class is going

So I tried to use core class with a basic Catalyst controller:

use Moose;
use MooseX::MethodAttributes;
use Example::Syntax;

class Example::Controller::Account :isa(Example::Controller) {

  sub root :At('$path_end/...') Via('../protected')  ($self, $c, $user) {
    $c->action->next($user->account);
  }

    sub prepare_edit :At('...') Via('root') ($self, $c, $account) { 
      $self->view_for('edit', account => $account);
      $c->action->next($account);
    }

      sub edit :Get('edit') Via('prepare_edit') ($self, $c, $account) {
        return  $c->view->set_http_ok;
      }

      sub update :Patch('') Via('prepare_edit') BodyModel ($self, $c, $account, $bm) {
        return $account->update_account($bm) ?
          $c->view->set_http_ok : 
            $c->view->set_http_bad_request;
      }
}

__PACKAGE__->meta->make_immutable;

But when I try to run I get a compile time error:

Class :isa attribute requires a class but "Example::Controller" is not one at lib/Example/Controller/Account.pm line 5.

I'm enabling the class pragma via the 'Example::Syntax' module , it basically does 'use experimental class'.

I would propose that code like this working should be considered mandatory before moving this out of experimental status.

[leonerd]

Class :isa attribute requires a class but "Example::Controller" is not one at lib/Example/Controller/Account.pm line 5.

I'm enabling the class pragma via the 'Example::Syntax' module , it basically does 'use experimental class'.

Yeah; the wording of the message isn't very clear but it's hard to explain without ending up writing most of a paragraph of text in the message. perldiag has more:

      Class :isa attribute requires a class but "%s" is not one
          (F) When creating a subclass using the class :isa attribute, the named superclass
          must also be a real class created using the class keyword.

I would propose that code like this working should be considered mandatory before moving this out of experimental status.

100% agree. This and about a hundred other as-yet-missing but definitely planned features are part of the reason it remains experimental, and is likely to for some time yet.

[jjn1056]

@Ovid

Here's the syntax class

package Example::Syntax;

use strict;
use warnings;

use Import::Into;
use Module::Runtime;

sub importables {
  my ($class) = @_;
  return (
    'utf8',
    'strict',
    'warnings',
    ['feature', ':5.38'],
    ['experimental', 'class', 'try', 'defer'],
  );
}

sub import {
  my ($class, @args) = @_;
  my $caller = caller;
  foreach my $import_proto($class->importables) {
    my ($module, @args) = (ref($import_proto)||'') eq 'ARRAY' ? 
      @$import_proto : ($import_proto, ());
    Module::Runtime::use_module($module)
      ->import::into($caller, @args)
  }
}

1;

[Ovid]

Things like this are exactly why we're proceeding cautiously. Consider the following:

#!/usr/bin/env perl

use v5.38.0;
use experimental qw(class);

class Person {
    field $name :param = 'John Doe';

    method name () {
        return $name;
    }

    sub name2 ($self) {
        return $name;
    }
}

That won't compile. It fails with: Field $name is not accessible outside a method at ...

That's because subroutines aren't methods and can't have access to internals. Later, it's likely that even if we can write code which compiles, we should at least get a runtime error if we try to call subroutines as methods because they're fundamentally different things. This is one of the reasons why boilerplate such as namespace::clean gets used so much: we shouldn't be exposing subroutines as methods.

Do you have the code for the Example::Controller?

[Ovid]

Also, the Moose stuff does a lot of heavy lifting with the MOP and appears to make assumptions about Moose being used. We can't just automatically inherit from non-Corinna classes right now because it's not clear that it would even work. A subclass should work with any code that accepts a parent class (Liskov) and it's not clear that's the case.

Thus, following our "Principle of Parsimony", we're deliberately being restrictive at first to make sure we can shake out all the bugs. Later, we can revisit opening things up. If we start with the latter and it turns out to be problematic, we might not be able to walk back that decision because working code might rely on it.

[jjn1056]

@Ovid I can't help shake out bugs if bug #1 is "I can't use it".

[Ovid]

The more I think about this, the more I realize a longer answer is warranted.
The following explains some of the design decisions.

Post-MVP this decision might be revisited, but for the MVP, we needed to
constrain the problem space. We don't know what a parent class does or how it
does it. Further, consider this:

class My::Class :isa(Other::Class) {
    field $name :param;

    sub get_name ($self) {
        return $name;
    }
}

The above is a syntax error because subroutines cannot access instance data.
Now consider this:

class My::Class {
    sub sum ($self,@nums) {
        my $total = 0;
        $total += $_ for @nums;
    }
}

That looks fine, right? Well, not really. Even though it's not accessing
instance data, if we allow that, we can no longer cleanly distinguish between
subroutines and methods. The following illustrates the problem:

class My::Class {
    use List::Util qw(sum);
}

That's similar, but it doesn't take an invocant ($self) as the first
argument. Ah, you say, just check to see if the sub is declared in the class?
Well, what about this?

class My::Class :does(Some::Role) {
    ...
}

If Some::Role was created with Role::Tiny or something, we'd be getting
subroutines instead of methods. We can't clealy tell if the subroutine is
supposed to be a sub or a method. We can jump through hoops to try to figure
that out, or we can just say "subroutines and methods aren't the same thing"
(which is true in many OO languages). That makes the OO code more efficient
and easier to program. We can safely import helper subroutines and not worry
about $object->can('sum') returning true for subs (this is not yet
implemented).

And that brings us back to our core problem:

class My::Class :isa(Other::Class) {
    field $name :param;

    sub get_name ($self) {
        return $name;
    }
}

If Other::Class produces blessed references, we can't safely know if
something is a callable method or not. That's why we often add extra
boilerplate such as use namespace::clean; to our code.

This isn't to say that we will never allow this, but for now, it was agreed
that it opens up a can 'o worms. The workaround, fortunately, is simple: use
composition over inheritance, a stalwart OOP guideline for decades.

class My::Class {
    use Other::Class;

    field $name :param;
    field $other_class = Other::Class->new( name => $name );

    method some_method ($arg) {
        return $other_class->some_method($arg);
    }
}

In the future, we might even be able to shorten that to this:

class My::Class {
    use Other::Class;

    field $name :param;
    field $other_class :handles(some_method) = Other::Class->new( name => $name );
}

But that means we don't get signature safety since signatures are not
introspectable. Trying to automatically mix the two object systems is a hard
problem, but when we manually write out the delegate, while tedious, it works
just fine.

Mixing Corinna and bless is like using motorcycle parts to fix a car. It might work,
but it probably won't and no guarantees are made.

Fortunately, class objects and blessed references can still safely call one another
and play together just fine.

[jjn1056]

@Ovid I favor composition over inheritance but for the use case I've linked in the GitHub repo that means there's not one single place I can use the new system. In Catalyst a control must inherit from Catalyst::Controller. I can't use core class for controller, for models, for view. I can't use it in DBIC for result or resultset classes either. It appears you can only use it for Greenfield code which is far from abundant in Perl.

[Ovid]

@Ovid I can't help shake out bugs if bug #1 is "I can't use it".

@jjn1056 I'm sorry you feel that way. Trying to retrofit old code onto the new class syntax isn't guaranteed to work. I appreciate that you're trying to shake out bugs, as said, mixing Corinna and bless is like using motorcycle parts to fix a car. It might work, but it probably won't and no guarantees are made.

If you wanted to write some brand-new code to shake out bugs, that's great.

Stevan Little has been doing so with Stella, building an actor model with class. He says he's quite pleased with the new system.

Chris "peregrin" Prather has been doing the same thing by building a clone of Rogue with the new syntax. He also appears to be happy with the new syntax.

[Ovid]

Oh, there's also the problem that Corinna is single-inheritance and bless is not, so that opens up nightmare of problems with the MRO. It would get even worse if we had a class object inherit from a blessed object which, in turn, inherited from a class object. Bugs 'o plenty.

We also recognize that class might ultimately need a different base class than UNIVERSAL (method can comes to mind). If we find that we need that, or that it offers so many benefits that we want that, what universal base class applies if class inherits from a blessed object?

I don't think there was anyone on the design team who was happy that the new object system can't inherit from the old, but the MVP is hard and trying to retrofit it onto the bless is a nightmare. Again, we might revisit this post-MVP, but not now.

I get that it's frustrating that you can't use this with existing bless systems. I wish we could, too.

[leonerd]

Need I remind folks that Object::Pad exists? ;)

Object::Pad can subclass an existing bless-based class just fine. It's an excellent testing ground to try out whether you like the new syntax.

It also makes a good migration path. Migrate a few bits of code at a time using it, until eventually all your codebase is using class. Then gradually migrate from Object::Pad to Feature::Compat::Class, thus putting you ready to try core's class feature, if you're on a new enough perl. Plus that also works even on perls before v5.38.

All it requires is not being afraid of CPAN modules.

[Ovid]

@jjn1056 I think @leonerd's comment about Object::Pad is the way to go. It's similar enough to what you're discussing that it's probably safer than Corinna. It's designed as a test bed for Corinna, but it also pushes much farther than the MVP does.

[nicomen]

Slightly related question, as I tried to see if I could "translate" an existing Moo* class on the fly to perl class syntax.

The following all fail, should or shouldn't we support dynamic class names?

use v5.38;
use feature 'class';
no warnings 'experimental::class';

my $class = 'Class';

my @tests = (
  'class $class {}',
  'class "My${class}" {}',
  'class $class 1.0 {}',
  'class "My${class}" 1.0 {}',
);

do { say "\n$_:\n"; eval "$_" or warn @_; } for @tests;

class $class {}:

Invalid version format (non-numeric data) at (eval 1) line 1, near "class "
syntax error at (eval 1) line 1, near "class $class "
Execution of (eval 1) aborted due to compilation errors.
	...caught at perl_class_test.pl line 16.

class "My${class}" {}:

Invalid version format (non-numeric data) at (eval 2) line 1, near "class "
syntax error at (eval 2) line 1, near "class "My${class}""
Execution of (eval 2) aborted due to compilation errors.
	...caught at perl_class_test.pl line 16.

class $class 1.0 {}:

Invalid version format (non-numeric data) at (eval 3) line 1, near "class "
syntax error at (eval 3) line 1, near "class $class "
Execution of (eval 3) aborted due to compilation errors.
	...caught at perl_class_test.pl line 16.

class "My${class}" 1.0 {}:

Invalid version format (non-numeric data) at (eval 4) line 1, near "class "
syntax error at (eval 4) line 1, near "class "My${class}""
Execution of (eval 4) aborted due to compilation errors.
	...caught at perl_class_test.pl line 16.

[Ovid]

@nicomen It's failing, in part, because you didn't interpolate the class names. Also, once a class is declared, you can't redeclare it with the same name. I think this is closer to what you want:

use v5.38;
use feature 'class';
no warnings 'experimental::class';

my $class = 'Class';

my @tests = (
    "class C1$class {}",
    "class C2My${class} {}",
    "class C3$class 1.0 {}",
    "class C4My${class} 1.0 {}",
);

do { say "$_:\n"; eval "$_; 1" or warn $@; }
  for @tests;

[nicomen]

@Ovid I intentionally do not want to interpolate until the actual perl code is eval()-ed. I just wanted to bulk the examples together. (And the different examples should perhaps have a bit different error messages.)

This example is perhaps clearer:

use v5.38;
use feature 'class';
no warnings 'experimental::class';

my $class = 'Class';

class $class {}

Invalid version format (non-numeric data) at perl_class_test.pl line 10, near "class "
syntax error at perl_class_test.pl line 10, near "class $class "
Execution of perl_class_test.pl aborted due to compilation errors.

This tells me that we are not allowed to set class names based on variables, my initial question is wether this should be possible or not.

[Ovid]

That's more of a question for @leonerd then. Not sure the fine-grained details of parsing there. Class names from variables will certainly be possible with the MOP, but that's not built yet.

[tonycoz]

my $class = 'Class';

class $class {}

One problem with this is the assignment is done at runtime, but the class is parsed and sealed at compile time.

[Ovid]

@tonycoz That's a good point. It will mean that dynamically creating new classes will need to be done through the MOP. When we get to anonymous classes, that will impact us, too.

[leonerd]

This is exactly the same as package:

$ perl -E 'my $pkg = "hello"; package $pkg {};'
Invalid version format (non-numeric data) at -e line 1, near "; package "
syntax error at -e line 1, near "package $pkg "
Execution of -e aborted due to compilation errors.