593277 - "Assertion failure: !vp->isPrimitive()"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Testcase:
  new (Proxy.createFunction({}, function(){}, function(){}));

Result:
  Assertion failure: !vp->isPrimitive(), at jscntxtinlines.h:632

The first bad revision is:
  changeset:   52720:66c8ad02543b
  user:        Luke Wagner <[e-mail]>
  date:        Mon Aug 16 12:35:04 2010 -0700
  summary:     Bug 581263 - remove slow natives (r=waldo,mrbkap)

Comment 1

[Luke Wagner [:luke]]

The problem is that native constructor calls no longer replace primitive return values with the original constructed 'this' -- there is no original constructed this.  So it falls on proxy_Construct to do the right thing, which I missed in the rm-slow-natives patch.

In doing the obvious thing (using js_NewInstance, which gives you the same 'this' object you got with slow native constructors), I had a question: was the old behavior the intended behavior? That is, since the callee is a Proxy, the default 'this' return value (e.g., in the testcase in comment 0) will be a Proxy object.  I don't know anything about proxies, but I would have guessed you would want the return value, if the handler returns a primitive, to be (new Object()).

Andreas or Brendan: any thoughts?  I didn't see any obvious answers on the Harmony Proxies page.

Comment 2

[Luke Wagner [:luke]]

Ah, Brendan said that this was discussed recently in committee and that proxies are indeed allowed to return primitives.  So, to keep the assertion (which I think is valuable), we can just change the assertion to an implication:

  JS_ASSERT_IF(native != proxy_Construct, !vp->isPrimitive());

Comment 3

[Brendan Eich [:brendan]]

FTR, http://wiki.ecmascript.org/doku.php?id=harmony:proxies_semantics#detailed_semantics_of_additional_behavior_for_function_proxies

/be

Comment 4

[Luke Wagner [:luke]]

Attachment: weaken asserts

This weakens the asserts to give proxies a pass.  This also fixes bug 593473.

Comment 6

[Luke Wagner [:luke]]

Attachment: fix

mrbkap was kind enough to point out that my previous patch commented out most of test_wrappers.xul :P

Comment 8

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/358b99602aea

Comment 9

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/358b99602aea