Ticket #1144 (closed bug: fixed)

Opened 5 years ago

Last modified 5 years ago

MMD incorrectly matches _ instead of String

Reported by: Austin_Hastings Owned by:
Priority: normal Milestone:
Component: none Version: 1.7.0
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

In the code below, I believe that calls to foo('bar') should be delivered to the :multi(String) sub, not the :multi(_) sub.

.namespace []
.sub 'foo' :multi(_)
	.param pmc args :slurpy
	print "multi(_) : arg is a: "
	$P0 = shift args
	$S0 = typeof $P0
	say $S0
.end

.sub 'foo' :multi(String)
	.param pmc args :slurpy
	say "multi(String)"
.end

.sub 'main' :main
	foo(1)
	foo('bar')
	foo('bar', 'baz')
	$P0 = box 'bar'
	foo($P0)
	foo($P0, 'baz')
.end

On my 1.7.0-devel version, the output is:

multi(_) : arg is a: Integer
multi(_) : arg is a: String
multi(_) : arg is a: String
multi(String)
multi(String)

Attachments

tt1144.patch Download (1.4 KB) - added by whiteknight 5 years ago.
patch to fix this issue

Change History

Changed 5 years ago by Austin_Hastings

After bouncing off this problem again, and doing some further investigation, I've learned that:

mmd_distance (src/multidispatch.c ~ line 770-790) explicitly checks if the args to the call are "int,float,string" and the callsig wants "Int,Float,String", and assigns that promotion a distance of 1.

mmd_distance then, _immediately_, checks if the args to the call are "any basic type" and the callsig wants "PMC" (aka '_'), in which case the distance is 1.

On the one hand, this is two ways of saying that "boxing has a distance of 1". But on the other hand it means that when you pass a 'string' (lowercase) to a (String) or to a (_) multisig, they both have a distance 1.

What I was trying to achieve with (_) was "if you haven't matched anything closer, use this error case". To this end, I think that while constant->PMC should not be "big distance", it should probably be greater than just 1.

Changed 5 years ago by whiteknight

patch to fix this issue

Changed 5 years ago by whiteknight

I've attached a quick patch that makes this test case execute correctly. Output is:

multi(_) : arg is a: Integer
multi(String)
multi(String)
multi(String)
multi(String)

I'm running tests now to verify that it doesn't break anything. If all tests pass and I hear no complaints, I will commit it.

Changed 5 years ago by whiteknight

All tests pass with this patch.

Changed 5 years ago by whiteknight

  • status changed from new to closed
  • resolution set to fixed

Fixed in r44986

Note: See TracTickets for help on using tickets.