Ticket #1790 (new bug)

Opened 4 years ago

Last modified 4 years ago

r48965 changes Boolean :multi dispatch from 2.6.0 behavior

Reported by: pmichaud Owned by:
Priority: normal Milestone:
Component: core Version: 2.7.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

The change of Boolean in r48965 changes multidispatch w/o a deprecation notice:

pmichaud@orange:/zip/parrot$ cat bool-1.pir
.sub 'main' :main
    $P0 = new ['Boolean']
    'foo'($P0)
.end


.sub 'foo' :multi(Integer)
    say 'foo(Integer)'
.end

.sub 'foo' :multi(_)
    say 'foo(_)'
.end
   
pmichaud@orange:/zip/parrot$ RELEASE_2_6_0/parrot bool-1.pir
foo(Integer)
pmichaud@orange:/zip/parrot$ trunk/parrot bool-1.pir
foo(_)
pmichaud@orange:/zip/parrot$ trunk/parrot_config revision
48981
pmichaud@orange:/zip/parrot$ 

This causes at least one Rakudo spectest to fail where it previously succeeded (S03-operators/short-circuit.t test 20).

Pm

Attachments

temporary_boolean_isa_integer.patch Download (1.3 KB) - added by NotFound 4 years ago.
Patch with a hackish temporary fix

Change History

  Changed 4 years ago by pmichaud

  • summary changed from r48965 changes :multi dispatch from 2.6.0 behavior to r48965 changes Boolean :multi dispatch from 2.6.0 behavior

  Changed 4 years ago by Paul C. Anagnostopoulos

We agreed that the new Boolean PMC did not require deprecation, but perhaps we were wrong. Let me investigate.

follow-up: ↓ 4   Changed 4 years ago by coke

Multidispatch is a bit of a red herring; this test is showing that Boolean no longer ISA Integer after this patch.

in reply to: ↑ 3   Changed 4 years ago by pmichaud

Replying to coke:

Multidispatch is a bit of a red herring; this test is showing that Boolean no longer ISA Integer after this patch.

For clarification: The ticket is primarily noting that a core behavior changed since 2.6.0 w/o a deprecation notice, and that :multi dispatch is where it manifests in Rakudo.

As far as Rakudo is concerned we can certainly accommodate the change in Boolean's parentage; but there probably needs to be a documented process for dealing with changes that fall afoul of the deprecation policy. In that case, this ticket is a placeholder for making sure this particular change gets documented somewhere appropriate.

Pm

  Changed 4 years ago by whiteknight

There's no real reason why we can't have the best of both worlds, at least for a full deprecation period. We can still have boolean ISA Integer, but use the new mechanism internally for data storage to save us on performance cost. At the very least we do need to support the old API behavior to avoid breakages in Rakudo and other end-users, but if we can fake that API while keeping the shiney new internals, more power to us.

Changed 4 years ago by NotFound

Patch with a hackish temporary fix

  Changed 4 years ago by NotFound

Something like this hackish patch?

follow-up: ↓ 8   Changed 4 years ago by Paul C. Anagnostopoulos

But if it doesn't actually inherit from Integer, have we solved the problem?

in reply to: ↑ 7   Changed 4 years ago by whiteknight

Replying to Paul C. Anagnostopoulos:

But if it doesn't actually inherit from Integer, have we solved the problem?

The boolean doesn't need to actually inherit from Integer, it just needs to have the same user-facing behavior as it had before. I think (without testing) that NotFound's patch should fool the MMD system into treating Boolean the same way as it was before. Can we get some kind of verification that this does work as intended?

  Changed 4 years ago by Paul C. Anagnostopoulos

Sorry, still pondering this. If someone uses an operation on a Boolean that is supported by Integer but not by Boolean (e.g., add), then we won't fix the problem if Boolean doesn't inherit from Integer.

  Changed 4 years ago by whiteknight

So in that case, create an add vtable in Boolean with a temporary implementation, and make sure it is listed as deprecated so we can rip it back out again when the time comes.

  Changed 4 years ago by Paul C. Anagnostopoulos

But how can we know what people are relying on?

Note: See TracTickets for help on using tickets.