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

One more approach for EXPORT problems. #74

Closed
vrurg opened this issue Jul 21, 2019 · 4 comments
Closed

One more approach for EXPORT problems. #74

vrurg opened this issue Jul 21, 2019 · 4 comments
Assignees
Labels
rakudo Big changes to Rakudo

Comments

@vrurg
Copy link
Contributor

vrurg commented Jul 21, 2019

What Is Wrong

The problem with EXPORT is best demonstrated by this example gist where running use.p6 results in:

Scalar
===SORRY!===
P6opaque: no such attribute '@!dispatchees' on type Routine in a Scalar when trying to get a value

The cause lies in the fact that symbols are exported using Hashes which containerize their values. By the moment mod3 is imported there is &trait_mod:<is> installed in use.p6 which is Scalar containing the dispatcher Sub, as follows from the error message the output of BEGIN block.

While I managed to partially address this issue with rakudo/rakudo#3016 but that PR is still more of a palliative but not the cure. After some discussion (though I don't remember exactly where it took place) I was convinced that deconting of &-sigiled symbols is not needed and even harmful. An example of why is it so would be:

sub foo is export { }
our &foo_alias is export = &foo;

Where &foo_alias is actually a writable scalar. Deconting it would break a code in a module consuming code:

&foo_alias = &my_foo;  # cannot change a read-only value

Proposed solution

Considering that the following use of EXPORT routine is now commonplace:

multi EXPORT {
    %(
        My::Other::Module::EXPORT::DEFAULT::, 
        '&foo' => &bar
    )
}

and by taking into account that containerization of Hash values is done on purpose and is a key part of the language definition, I see the following as a solution with the least need amount of changes to both the core and the user land while improving overall DWIM logic:

  1. Stash behavior changes in a way that assigning to a key doesn't containerize the value. Basically, assignment and binding become the same. I.e.:

    my %h is Stash = '&foo' => &foo;
    say %h<&foo>.VAR.^name; # Sub; now it'd be Scalar
    
  2. Stash has to become the de-facto standard of EXPORT return value. I would even make it a warning if Hash is returned.

  3. Because

    multi EXPORT { 
        Stash.new: My::Other::Module::EXPORT::DEFAULT:: 
    }
    

    is somewhat cumbersome, I propose a new operator ( ):: Which follows the common logic of :: used to access symbol tables. Though in this case it'd be used to create one:

    multi EXPORT {
        ( My::Other::Module::EXPORT::DEFAULT:: )::
    }
    

    A sample implementation of the operator can be found in the same gist in file mod3.pm6.

With this proposal there will be no more need to guess if &foo is the actual routine or a variable aliasing a routine.

Affected Cases

I suspect that a number of 'Cannot invoke this object (REPR: Null; VMNull)' or 'No such method 'CALL-ME' for invocant' issues could be caused the this problem of EXPORT. Not necessarily by re-exporting via &EXPORT but could be indirectly caused by manipulations with a Stash object.

@AlexDaniel AlexDaniel added the rakudo Big changes to Rakudo label Jul 21, 2019
vrurg added a commit to vrurg/rakudo that referenced this issue Jul 22, 2019
Turn off containerization for Stash

Also added operator (<EXPR>):: which is a shortcut for
Stash.new(<EXPR>)

Now during import values from Stash are used as-is without deconting.

Raku/problem-solving#74
@vrurg
Copy link
Contributor Author

vrurg commented Jul 22, 2019

PR rakudo/rakudo#3076 implements this proposal. Passes all tests. No tests for the PR itself are added yet as I'm not certain if this proposal would be accepted.

With the above PR the example in gist works as expected if Stash.new or ():: operator is used in EXPORT. It also fixes a problem with my internal project which uses Red framework and was suffering from 'No such method 'CALL-ME' for invocant' error.

@lizmat
Copy link
Collaborator

lizmat commented Jul 24, 2019

The BIG problem with rakudo/rakudo#3081 is that it makes Map to no longer be a value type, but it becomes an object type.

@vrurg
Copy link
Contributor Author

vrurg commented Jul 29, 2019

@lizmat proposed and implemented with rakudo/rakudo#3081 non-decontainerizing Map which actually provides the ultimate solution to the export problem. If no unwanted side effects would pop up then this problem could be closed later.

vrurg added a commit to vrurg/Red that referenced this issue Jul 29, 2019
FCO pushed a commit to FCO/Red that referenced this issue Jul 29, 2019
@vrurg
Copy link
Contributor Author

vrurg commented Aug 19, 2019

Closed via rakudo/rakudo#3081.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rakudo Big changes to Rakudo
Projects
None yet
Development

No branches or pull requests

4 participants