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

Add dispatch disambiguation via 'is item' parameter trait #5549

Merged
merged 5 commits into from Apr 14, 2024
Merged

Conversation

ab5tract
Copy link
Collaborator

@ab5tract ab5tract commented Apr 6, 2024

This allows for multi routines to distinguish between itemized arguments
via a new is item parameter trait.

For the sake of simplicity, the parameter trait does not add any new
ParamTypeChecks1 to the parameter. Instead it is only used as a marker
for disambiguation in multiple dispatch:

    multi sub a(@a is item) { "itemized!" }
    multi sub a(@a) { "array" }

    a [1,2];  # "array"
    a $[1,2]; # "itemized!"

Note that this only makes sense for @ (positional) and % (associative)
sigilled parameters. Raw, unsigilled parameters are also currently included,
but only if the parameter type is specified to something that does Positional
or Associative.

Footnotes

  1. Taking this approach is complicated by the fact that there appears to be
    some decontainerization of arguments via the callstatic and callmethod
    ops: a ParamTypeCheck which calls iscont on the lowered param always
    fails for $[] and ${} arguments. Further research is required.

@ab5tract ab5tract force-pushed the is-item-redux branch 2 times, most recently from 0684f1c to b1d699c Compare April 8, 2024 16:10
@raiph
Copy link
Contributor

raiph commented Apr 9, 2024

Hi @ab5tract,

Has there been discussion I could read about the use cases and possible solutions?

Some code and related thoughts:

multi sub a($a) { $a, VAR $a }
multi sub a(@a) { "array" }
say a VAR $[1,2]; # ([1, 2] $[1, 2])

Here I've modified the call site. But I've long wondered why the semantics of $ weren't VAR $. If that change were made for 6.e then one could use a $ sigil'd parameter as shown. It's not the same as what you're suggesting, but perhaps it's better?

@ab5tract ab5tract force-pushed the is-item-redux branch 2 times, most recently from 5bcd298 to 2d024c4 Compare April 11, 2024 15:53
Since we already visit all of the parameters here, we
can avoid type and flag checks of these same parameters
in the dispatch code by providing an `item_disambiguation`
flag in the dispatch_info hash that we access later in
the multi dispatch logic.

This is a relatively heavy intervention into the way that
dispatch normally works. It is all to workaround the current
reality of 'callmethod' and 'callstatic' having a bug whereby
the argument passed to ParamTypeCheck is already decont'd.

Since the entire point of `is item` is to dispatch based on
containerization, that's clearly a blocker.

So, in the case of a named parameter with an in `is item` trait
we cannot simply rely on the VM binder to check the candidate's
suitability. Instead we pass the type information of `is item`
named argumants along with the rest of the dispatch_info hash.
This allows for multi routines to distinguish between itemized arguments.

For the sake of simplicity, the parameter trait does not add any new
ParamTypeChecks to the parameter. Instead it is only used as a marker
for disambiguation in multiple dispatch:

    multi sub a(@A is item) { "itemized!" }
    multi sub a(@A) { "array" }

    a [1,2];  # "array"
    a $[1,2]; # "itemized!"

This also works for associatives (${} / {}) and lists ($() / ()).
Sigil-less parameters with Associative or Positional types can also
be disambiguated by 'is item'.

It also works on named parameters. Note that nested-parameters are
currently not parsing with traits at the moment. When they do, this
code will require some adjustment as currently we only use one name
in our name-to-type lookup. Since there was no way to test that it
would work, it has been left for later.
This adds param.is-item disambiguation to the legacy dispatcher.
See commit 11ae695 along with R#5549 for more information.
@ab5tract
Copy link
Collaborator Author

@raiph Thanks for your comments! This is a follow-up/re-implementation of the behavior previously merged in #5543. The use case that has driven the development is to resolve bug report #5500. Discussions were logged over the last 3 or so weeks in #raku-dev.

The solution as implemented here is designed to be the least invasive / most agreeable in relation to keeping the base dispatch behavior while allowing multi dispatch designs to incorporate a distinction between positionals/associatives and their itemized forms (whether that is actually good API design is a different question).

Having dug around in the dispatcher quite a bit recently, I don't think there is an easy way to implement your proposal. It's also a significant change to the semantics of Raku. That's not a bad thing but it is beyond the scope of this MR's intentions.

@ab5tract ab5tract merged commit c02bd13 into main Apr 14, 2024
9 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants