Ticket #1144 (closed bug: fixed)

Opened 12 years ago

Last modified 12 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 12 years ago.
patch to fix this issue

Change History

Changed 12 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 12 years ago by whiteknight

patch to fix this issue

Changed 12 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 12 years ago by whiteknight

All tests pass with this patch.

Changed 12 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.