Implementation of '__noSuchMethod__' handler for SpiderMonkey

VERIFIED FIXED in mozilla1.6alpha

Status

()

Core
JavaScript Engine
P4
enhancement
VERIFIED FIXED
15 years ago
9 years ago

People

(Reporter: William J. Edney, Assigned: brendan)

Tracking

Trunk
mozilla1.6alpha
x86
All
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 7 obsolete attachments)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.2) Gecko/20021216
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.2) Gecko/20021216

ActionScript, as you may or may not know, is an "ECMAScript-like" language that
is a little too underpowered IMHO (no 'real' eval(), no Function constructor, no
RegExps), but it does has one capability that caught my eye: a way to trap
unresolved slots. They do this via their private '__resolve' function:

MyObjType.prototype.__resolve = function (slotName)
{
alert('hey pal... you tried to access: ' + slotName + ' and that doesn't exist
on this object');
};

So, that if you do 'myObj = new myObjType(); myObj['foo']', and it doesn't have
a 'foo' slot, the '__resolve' function is called.

According to their docs, this also works for function calls:

MyProxyType.prototype.__resolve = function (funName)
{
// forward to our real obj...
return this.realObj[funName].apply(this.realObj,arguments);
};

This, then, acts much as Smalltalk's 'doesNotUnderstand:' method or
Objective-C's 'forward::' method. It allows forwarding of methods that are not
understood by that object to other objects in the system (or you could put debug
logging in there, etc.).

Adding this capability to Spidermonkey would allow us to remove a bunch of code
that we currently hack in in TIBET to supply a DNU hook. Also, it's the one
feature I've found in ActionScript that Spidermonkey doesn't have ;-). 

Reproducible: Always

Steps to Reproduce:
1.
2.
3.




Here's the page from the ActionScript manual, copied without permission ;-).

Although this doesn't specifically say they do this 'undefined' redispatch for
functions, I've seen it used that way in folk's examples around the Net. The
power, as we see it, is the ability to put a "generic backstop" '__resolve' hook
on Object.prototype and then 'override' that down the prototype chain, if we
wanted to do other things for other types.

Availability

Flash Communication Server MX.

Usage

Client.__resolve = function(propName){
	// insert code here
};

Parameters

propName The name of an undefined property.

Returns

The value of the undefined property, which is specified by the propName parameter.

Description

Method; provides values for undefined properties. When an undefined property of
a Client object is referenced by server-side ActionScript code, that object is
checked for a __resolve method. If the object has a __resolve method, the
__resolve method is invoked and passed the name of the undefined property. The
return value of the __resolve method is the value of the undefined property. In
this way, __resolve can supply the values for undefined properties and make it
appear as if they are defined.

Example

The following example defines a function that is called whenever an undefined
property is referenced:

Client.prototype.__resolve = function (name) {
	return "Hello, world!";
};
function onConnect(newClient){
	trace (newClient.property1); // this will print "Hello World"
}

Comment 1

15 years ago
Reassigning; cc'ing Brendan, Waldemar to consider this request.
Assignee: rogerl → khanson
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

15 years ago
I might be interested in adding this to the monkey.  Probably something similar
to __defineGetter__ and __defineSetter__:

obj = new Object();
obj.prototype.__resolve__ =
function resolveFunc(propName)
{
     switch (propName)
     {
          case "prop":
              return this._prop;

          case "someMethod":
              return this._method;
     }
}

and...

obj = {
    resolve resolveFunc(propName) { ... }
};

Comment 3

15 years ago
err, remove the ".prototype" reference from my previous comment.
(Assignee)

Comment 4

15 years ago
I'd rather not add the resolve foo() {...} property initializer syntax, if
__resolve__ will do.  The SpiderMonkey patch for the latter is trivial.

/be

Comment 5

15 years ago
The only issue with not including the resolve foo() syntax is that
Object.prototype.toSource() will not be able to produce an accurate
representation of the object without it.
(Assignee)

Comment 6

15 years ago
Doing this without badly hurting performance is less trivial.

/be
(Assignee)

Comment 7

15 years ago
Re: comment #5: I don't see the problem (and this is a real copy/paste):

[~/src/trunk-opt/mozilla/js/src]$ Linux_All_DBG.OBJ/js
js> Object.prototype.__resolve__=function(id){print('Resolving ' + id)}

function (id) {
    print("Resolving " + id);
}

js> uneval(Object.prototype)
({__resolve__:(function (id) {print("Resolving " + id);})})

Highly questionable patch in a moment.

/be

Comment 8

15 years ago
duh.  What was I thinking.
(Assignee)

Comment 9

15 years ago
Created attachment 116357 [details] [diff] [review]
patch implementing __resolve__ for native objects

The only good thing about this patch is that it's small.  It adds overhead only
to js_SetProperty, and then only an atom comparison.  But this patch makes it
very easy for you to hose yourself (e.g., with Object.prototype.__resolve__ =
function(id) { this[id] = 42 }).

/be
(Assignee)

Comment 10

15 years ago
Comment on attachment 116357 [details] [diff] [review]
patch implementing __resolve__ for native objects

This patch also doesn't work for classes that set JSCLASS_NEW_RESOLVE -- the
property never resolves, no matter what the __resolve__ function does.

/be
Attachment #116357 - Attachment is obsolete: true
(Assignee)

Comment 11

15 years ago
Comment on attachment 116357 [details] [diff] [review]
patch implementing __resolve__ for native objects

The worst thing about this patch is that we clobber the JSClass.resolve hook
for all instances, even if script wants to specialize a resolve function for
just one instance.

Ok, I'm done beating up on this patch -- it's marked obsolete already!

/be

Comment 12

15 years ago
Created attachment 116375 [details] [diff] [review]
the naive approach

This patch looks up the "__resolve__" property when a lookup fails.  It incurs
a second lookup for undefined properties, which is probably substantial
additional overhead when __resolve__ isn't defined, and may not be acceptable.

Because the approach is simple minded, I assume it's safe.  But frankly I don't
know enough about the engine to say why we don't die a recursive death when
__resolve__ itself isn't defined.

Comment 13

15 years ago
Created attachment 116376 [details]
testcase

js testcase for resolve hook

Comment 14

15 years ago
cc'ing Igor in case this idea would be of interest to Rhino -
(Assignee)

Comment 15

15 years ago
Comment on attachment 116375 [details] [diff] [review]
the naive approach

This affects only native property gets, not sets.  I think the user-defined
resolve hook should be called exactly where the JSClass one is, so that it
handles both cases.

Also, nits: don't re-atomize commonly used strings, do use ID_TO_VALUE when
inside the engine (and thereby avoid fallible-API false test and early return),
and do use js_InternalInvoke instead of JS_CallFunctionValue, if you must use
either.

/be

Comment 16

15 years ago
Comment on attachment 116375 [details] [diff] [review]
the naive approach

I disabled js strict warnings and ran the following function three times, both
with and without the patch...

      function simpleTest()
      {
	 var obj = new Object();
	 for (var i = 0; i < 1000; ++i)
	     var x = obj.foo;

      }

According to Venkman, the numbers from before are...

    Total Time: 7.35 (min/max/avg 2.44/2.47/2.45)

And after...

    Total Time: 16.23 (min/max/avg 4.78/5.75/5.41)


More than a twice as slow.  Not very good.
Attachment #116375 - Attachment is obsolete: true
(Assignee)

Comment 17

15 years ago
Questions: when an object is an instance of a native class that has a non-stub
resolve hook, and the object has had a __resolve__ property set to a function
object value, which should be called?

1) the function object stored in the object's __resolve__ property;
2) the class resolve hook;
3) first 1, then 2.

The only way to make all this work is to keep track efficiently -- in a way that
js_LookupProperty can test very quickly -- of which objects have had __resolve__
properties set, and treat only those specially.  That takes more work, which I'm
thinking about how to do best (without adding a lot of code bloat for this feature).

/be
Assignee: khanson → brendan
Priority: -- → P4
Target Milestone: --- → mozilla1.4alpha

Comment 18

15 years ago
Option 2 sounds correct to me.
(Assignee)

Comment 19

15 years ago
Option 2 says that __resolve__ does not override the class resolve hook.  Why
would that be desirable?

If __resolve__ means the same thing as JSClass.resolve, then users can customize
per-object, but the question I raised (first of several?  I forget why I used
Questions-plural) was should the __resolve__ hook run first, then the class
resolve hook?  Or should __resolve__ replace the class resolve hook?  IOW, I
don't like answer 2, because it means __resolve__ is either useless, or else
different from the class resolve hook.

So how about 1 vs. 3?

I know how to implement this efficiently, but it requires a bigger patch, if not
a lot of compiled code.

/be
(Reporter)

Comment 20

15 years ago
Brendan -

As an end user who writes only JavaScript (although, I must admit, rather
aggressively ;-)), and knows virtually nothing about SpiderMonkey's
implementation details, when I hear the words 'per-object customization' that
sounds closer to my original intent when requesting this feature.

I would expect that, if a slot wasn't found on a particular object, that the
prototype chain would be traversed such that I could put a '__resolve__' on
Object.prototype that could act as a 'generic backstop' for the entire system
(including slot accesses against public native objects, like Array, Date, etc.)
while also allowing me to 'override' this behavior further down the chain (also
including being able to put a '__resolve__' on Date, etc.).

Sorry in advance if the signal-to-noise ratio on this post is too high, just
thought I'd throw in my uneducated 2 cents ;-).

Cheers,

- Bill
(Assignee)

Comment 21

15 years ago
Since this is a SpiderMonkey-only extension, I am exposing all the capabilities
of JSNewResolveOp/JSCLASS_NEW_RESOLVE  An example should make things clear:

var bar_delegate = {bar: 'bletch!'};

Object.prototype.__resolve = function (id, flags) {
  print('Resolving ' + id + ' with flags ' + flags);
  if (flags & Object.RESOLVE_QUALIFIED)
    print('the ' + id + ' reference was qualified by a . or [] expression');
  if (flags & Object.RESOLVE_ASSIGNING)
    print('the ' + id + ' reference is the left-hand side of an assignment');

  if (id == 'foo') {
    this[id] = 42;              // resolve the magic foo property
    return this;
  }

  if (id == 'bar') {
    return bar_delegate;        // 'bar' delegates to bar_delegate
  }

  return null;
}

Because __resolve__ overrides the class resolve hook, the recursion protection
in js_LookupProperty applies, too.  Patch in a second.

/be
Status: NEW → ASSIGNED
(Assignee)

Comment 22

15 years ago
Created attachment 116487 [details] [diff] [review]
large-looking patch, but it doesn't add much code

Still, added code is feature bloat.  Is this feature worth the cost?  That's an
apples-to-oranges problem, and I invite comments.  To quantify the cost, I
built with RH8's gcc (3.2) and -O2.  I see an increase of 268 bytes to the js
shell's text segment size with the patch, compared to without.

There is also a slight hit to native property setting and deleting, because of
the (id == (jsid) rt->atomState.resolveAtom) special case tests, but it's
likely to be in the noise.

To make all this work without too much overhead, I resuscitated part of a patch
from one of my many trees that moves the SCOPE_MIDDLE_DELETE_TAG from being a
low-pointer-bit tag on scope->lastProp to being a proper flag in scope->flags. 
The scope->flags member reclaims space given up by eliminating scope->sizeLog2
(but flags is at a different offset; I made it come before hashShift), so the
JSScope struct does not increase in size.  The cost of this change is an extra
subtract-from-immediate (JS_DHASH_BITS - hashShift) in the had-a-collision
"slow path" in SearchScope, so the cost in code and cycles is likely in the
noise.

Now that scopes have flags, a new SCOPE_RESOLVE_MAGIC flag can be set in o's
scope whenever 'o.__resolve__ = someFunction' is executed.  We know that o will
have its own scope, to store the __resolve__ property mapped to a slot in o, so
there's no gratuitous scope overhead introduced here.  We just take advantage
of the scope's flags member to note that js_LookupProperty should substitute
JS_ResolveMagic for the class resolve hook, henceforth.

Deleting o.__resolve__ clears the SCOPE_RESOLVE_MAGIC flag.  So does assigning
any non-function value to o.__resolve__.

JS_ResolveMagic is unconditionally added to the API, in case a native JSClass
outside of the engine wants to use it instead of JS_ResolveStub or a resolve
hook that would reinvent this wheel.

Comments from JS language users and gurus, and from SpiderMonkey implementation
experts, are welcome.

/be
(Assignee)

Comment 23

15 years ago
Er, comment #21 should read:

Object.prototype.__resolve__ = function (id, flags) {

(Trailing __ after __resolve, curse the ActionScript variant for confusing me! ;-)

/be
(Assignee)

Updated

15 years ago
Attachment #116487 - Flags: review?(shaver)

Comment 24

15 years ago
As I understand it, option 2 doesn't make __resolve__ useless, it just removes
the script resolve hook from the loop for native objects with a non stub native
resolve hook.   document.__resolve__ might not work, but o = { __resolve__:
function () {...}} would still be plenty useful.  If a native object with custom
resolve hook wants to explicitly allow scripts to make use of __resolve__, it is
free to call out to JS_ResolveMagic.

Providing script the option to completely replace the native resolve hook sounds
like it's asking for trouble.  Without access to the native resolve hook for
Components.classes, for example, the object becomes useless.
(Reporter)

Comment 25

15 years ago
Brendan -

As the originator of this request, it should be no surprise that I will put
forth a 'for' argument for this feature ;-):

At my company, we've pushed this language to its limits (and some would say
beyond ;-)) and have always been amazed at the flexibility it afforded us to
mostly do what we wanted to do. We have a framework that does some pretty
incredible things, all due to the power of JS.

This feature is one we have sorely missed, however, and, although we have our
own hack that allows us to simulate a 'pseudo' __resolve__-like functionality,
it takes up quite a bit of memory and loads up Object.prototype with a bunch of
additional keys. Having __resolve__ would allow us to remove all of that for
Mozilla.

Every dynamic language that I've worked/played with has had some sort of
functionality such as __resolve__ exposed to the end programmer, and this has a
variety of uses, such as implementing the "Proxy" pattern, providing debug
information, etc. Smalltalk has 'doesNotUnderstand', Objective-C has 'forward',
Python has '__getattr__'/'__setattr__', Ruby has 'method_missing', Perl has
'AUTOLOAD', Tcl has 'unknown'.

We have found that, even by using our current 'hack', that we were able to
reduce code size substantially because we were able to reroute messages to
objects that could understand the message and take action. The ability to use a
native hook to do this would substantially increase speed and error-trapping
robustness for us.

I humbly submit that, at this point, this is an even bigger feature than
bringing back 'arguments.caller' ;-).

Cheers,

- Bill

Comment 26

15 years ago
I skimmed over the various patches but haven't checked them in detail.  I'm
curious about a couple things:

What's the interaction between this feature and variable lookup?  The early
patches didn't seem to affect it; the later ones do.

What are the security implications of doing this?
(Assignee)

Comment 27

15 years ago
Created attachment 116571 [details] [diff] [review]
just the jsdbgapi.c and jsscope.[ch] parts of attachment 116487 [details] [diff] [review]

I'd like to get the scope->flag change in; it will pay off in the future, e.g.,
when implemented sealed scopes for readonly standard objects that can be shared
safely among many threads.

/be
(Assignee)

Comment 28

15 years ago
Comment on attachment 116571 [details] [diff] [review]
just the jsdbgapi.c and jsscope.[ch] parts of attachment 116487 [details] [diff] [review]

Executive summary:
- Remove sizeLog2 from JSScope, deriving it via JS_DHASH_BITS -
scope->hashShift as needed.
- provide missing SCOPE_CAPACITY(scope) macro to compute the power-of-two scope
hash table size in entries (so old JS_BIT(scope->sizeLog2) uses in several
places can avoid knowing about JSScope internals).
- Move SCOPE_MIDDLE_DELETE_TAG into the new scope->flags member, as a flag bit,
SCOPE_MIDDLE_DELETE.  Be sure to clear this flag explicitly now!  The tag used
to be cleared implicitly, as a side effect of assigning to scope->lastProp.
- Add SCOPE_RESOLVE_MAGIC flag for use in rest of complete patch for this bug.

/be
Attachment #116571 - Flags: review?(shaver)
(Assignee)

Comment 29

15 years ago
Re: Rob's comment #24: you're proposing that __resolve__ override only the stub
class resolve hook, and not any non-stub class resolve hook.  That's easy to do,
but I wonder if alternative 3 isn't even better: try __resolve__ if set, then if
the property was not defined by that hook, try the class resolve hook (unless
it's a stub, of course - that's just an optimization).

Why shouldn't I be able to use __resolve__ with document or Components.classes,
to augment any built-in class resolve hook?

Re: Waldemar's comment #26: both of the patches I attached here affect global
variable lookup in the same way: for a browser embedding, if you manage to set
window.__resolve__ to a function, global variables will be looked up and, if not
found in the window object, window.__resolve__ wil be called, and may define the
sought-after property.

This does mean that window.__resolve__ should not be something an evil.org page
can set on any victim.com's window object.  Security implication!  Thanks,

/be

Comment 30

15 years ago
> Why shouldn't I be able to use __resolve__ with document or Components.classes,
> to augment any built-in class resolve hook?

I don't know.  I just assumed someone would come up with a valid reason to want
to prevent script from overriding their native resolve hook.  For security
reasons, for example.  I suppose they could still do that with a custom property
setter that ignored any attempt to set __resolve__, right?
(Assignee)

Comment 31

15 years ago
Security is handled by separate means from those that govern whether and how, in
a single trust domain, __resolve__ overrides or augments the class resolve hook.
 If it augments, everyone wins, provided we check access as we do for other
operations across trust domains.

/be
(Assignee)

Comment 32

15 years ago
Created attachment 116653 [details] [diff] [review]
the rest of the patch, with __resolve__ augmenting the class resolve hook, and with a security check per __resolve__

The main change is to have __resolve__ augment the class resolve hook, not
replace it.  Thus __resolve__ may override or yield to the class resolve hook,
depending on the id.

Note that the access check per __resolve__ attempt means that evil.org could
set victimWindow.__resolve__ = function(id,flags){log(id,flags);return null} to
try to spy, but the resolve attempts would fail with a security exception. 
This would not merely frustrate evil.org, as we want -- it would break
victimWindow pretty completely.

Instead, we could check access only when __resolve__ is being set, but that
means all ids are barred from being resolved, or none is barred.  It would also
require some changes to nsDOMClassInfo::CheckAccess, in dom/src/base -- not a
problem, just something that would need to be done (the JSACC_WRITE case for
the '__resolve__' id would need a special case there).

Comments welcome.

/be
Attachment #116487 - Attachment is obsolete: true
(Assignee)

Comment 33

15 years ago
Created attachment 116664 [details] [diff] [review]
last patch, take 2

Test mozilla first, then attach patch....

/be
Attachment #116653 - Attachment is obsolete: true
(Reporter)

Comment 34

15 years ago
Brendan -

Usage question:

to 'redispatch' a method, I assume I would slice the first two arguments ('id'
and 'flags') off of 'arguments' to run the apply? Like so...

Object.prototype.__resolve__ = function (id,flags)
{
return this.anotherObj[id].apply(this.anotherObj,arguments.slice(2));
};

Thanks!
(Assignee)

Comment 35

15 years ago
Bill: I don't know how __resolve can possibly work the way comment #0 implies,
both resolving a missing property and calling a function named by the missing
property's id.

The __resolve__ extension patched here does just the first, by allowing the
user-defined hook to define a property on the object in which __resolve__ was
found -- or in a delegate (the returned object).  If that new (or existing, in
the case of a delegate) property has a function as its value, and the expression
causing the resolve hook to be called is a call expression, then the function
will be called after __resolve__ unwinds.

This reminds me: we need to pass the "start" object, the one to the left of . or
[ where the property id to the right of . or in the [] triggers the __resolve__,
to the hook.  The |this| parameter is the object in which __resolve__ was found,
so if you set Object.prototype.__resolve__ = function (id){this[id]=42;return
this}, and then say o={}; o.moo, you'll see o.moo === 42, but 'moo' was defined
 in Object.prototype, not in o.

More patching soon-ish.

/be
(Assignee)

Comment 36

15 years ago
Created attachment 116796 [details] [diff] [review]
take 3

The previous patch busted because XPC_WN_Helper_NewResolve does not null its
*objp result parameter in the not-resolved case.  For backward compatibility
this patch adds a JSCLASS_NEW_RESOLVE_GETS_START flag, telling
js_LookupProperty to pass the "start" object (the one at the head of the
prototype chain linking in 0 or more steps to the object on which the
JSClass.resolve hook is currently being called) in via *objp.  Otherwise, as
before, *objp comes in null.

/be
Attachment #116664 - Attachment is obsolete: true
(Assignee)

Comment 37

15 years ago
Comment on attachment 116796 [details] [diff] [review]
take 3

Just for grins, I'm trying to get jband to sr= this patch.  I have no idea
whether he has time for a look, but I'd appreciate another pair of eyes.

Shaver's busy today, but I hope he'll get to this soon.

The open issue, in my view, is the per-__resolve__-call security check.  See
comment 32.

/be
Attachment #116796 - Flags: superreview?(jband)
Attachment #116796 - Flags: review?(shaver)
(Assignee)

Comment 38

15 years ago
Comment on attachment 116796 [details] [diff] [review]
take 3

One more fix, to the recursion suppression in js_LookupProperty, coming up.

/be
Attachment #116796 - Attachment is obsolete: true
Attachment #116796 - Flags: superreview?(jband)
Attachment #116796 - Flags: review?(shaver)
(Assignee)

Comment 39

15 years ago
Created attachment 116807 [details] [diff] [review]
take 4

The runaway recursion prevention code in js_LookupProperty always had this bug:
it would bail with null out parameters (goto out) when such recursion was
detected, instead of trying the next object on the prototype chain.  So now we
have goto next instead of goto out.  Plain diff with new-on-the-left against
last patch shows the change:

400c400
< @@ -2173,63 +2186,80 @@ JS_FRIEND_API(JSBool)
---
> @@ -2173,53 +2186,72 @@ JS_FRIEND_API(JSBool)
484,498c484,486
< -		       if (JS_DHASH_ENTRY_IS_BUSY(entry)) {
< -			   JS_UNLOCK_OBJ(cx, obj);
< -			   goto out;
< -		       }
< +		       if (JS_DHASH_ENTRY_IS_BUSY(entry))
< +			   goto next;
<		   } else if (!(table = cx->resolvingTable)) {
<		       table = JS_NewDHashTable(&resolving_dhash_ops,
<						NULL,
<						sizeof(JSResolvingEntry),
<						JS_DHASH_MIN_SIZE);
<		       if (!table)
<			   goto outofmem;
<		       cx->resolvingTable = table;
< @@ -2252,29 +2282,35 @@ js_LookupProperty(JSContext *cx, JSObjec
---
>		       if (JS_DHASH_ENTRY_IS_BUSY(entry)) {
>			   JS_UNLOCK_OBJ(cx, obj);
> @@ -2252,29 +2284,35 @@ js_LookupProperty(JSContext *cx, JSObjec
538c526
< @@ -2295,18 +2331,23 @@ js_LookupProperty(JSContext *cx, JSObjec
---
> @@ -2295,18 +2333,23 @@ js_LookupProperty(JSContext *cx, JSObjec
563c551
< @@ -2316,21 +2357,41 @@ js_LookupProperty(JSContext *cx, JSObjec
---
> @@ -2316,21 +2359,41 @@ js_LookupProperty(JSContext *cx, JSObjec
606c594
< @@ -2340,34 +2401,26 @@ js_LookupProperty(JSContext *cx, JSObjec
---
> @@ -2340,24 +2403,16 @@ js_LookupProperty(JSContext *cx, JSObjec
623d610
< +    next:
632,642c619
<
< -out:
<      *objp = NULL;
<      *propp = NULL;
<      return JS_TRUE;
<  }
<
<  JS_FRIEND_API(JSBool)
<  js_FindProperty(JSContext *cx, jsid id, JSObject **objp, JSObject **pobjp,
<		   JSProperty **propp)
< @@ -2684,16 +2737,31 @@ js_SetProperty(JSContext *cx, JSObject *

/be
(Assignee)

Updated

15 years ago
Attachment #116807 - Flags: superreview?(jband)
Attachment #116807 - Flags: review?(shaver)

Comment 40

15 years ago
Sorry I'm late to the party here, but it seems to me that the semantics of the
patch as described aren't consistent with the "doesNotUnderstand" functionality
we were trying to get with this feature request. Probably our fault for not
being as specific as possible with our request. 

Our basic requirement is that we'd like a way to put a global function in  (call
it __DNU__), that is called any time a Function invocation is done targeting a
function that doesn't exist. The __DNU__ function's signature would be:

function __DNU__(targetedObject, functionName, argumentsObject);

Implementors can then do whatever is appropriate to handle the functionality
that was being requested. That might be done ala: 

targetedObject.someOtherFunc(argumentsObject.arguments[0], ...);

It might also mean dispatching to a delegate, or simply reporting a cleaner
error that might include a stack dump acquired via the arguments.caller
properties up the call chain. 

Basically this is the "last chance hook" a developer can use before the engine
prints a message ala "targetedObject.functionName" is not a function". That's
pretty much the entire feature in a nutshell. If the __DNU__ function is defined
it's results are passed back just as if the original function had been found.
That functionality is what I think Bill was trying to point to with his
references to Smalltalk's doesNotUnderstand and Objective-C's forward methods.

While there is certainly value in a more general purpose "global setter/getter"
feature that's not really what we were looking for here. Not that we're unhappy
to get it as well :). But even then, I think it would be more useful not to
impose the semantics of "pass me back a delegate". Instead, I think it would be
far better to simply allow the implementer to return whatever value they'd like
to return as if the slot had actually existed.

My apologies if I'm misunderstanding the patch at this point. 

ss


The original request very much speaks of an object-specific hook, which is good,
because I think it's the only way this sort of thing will happen in JS.

|obj.meth(arg)| is two operations, in JS (and ECMA): get |obj.meth|, and then
invoke that function with |obj| as its |this|.

So:

Object.prototype.__resolve__ =
function (id)
{
    var originalObj = this; // for posterity
    function __DNU__()
    {
        print("I don't understand " + id + "(" +
              Array.prototype.join.call(arguments, ", ") + ")");
    }
    return __DNU__;
}

I can't make that work, for some reason, though:

js> o2.__resolve__

function resolver(id) {
    var originalObj = this;

    function __DNU__() {
        print("I don't understand " + id + "(" +
Array.prototype.join.call(arguments, ", ") + ")");
    }

    return __DNU__;
}

js> f = o2.__resolve__("a")

function __DNU__() {
    print("I don't understand " + id + "(" +
Array.prototype.join.call(arguments, ", ") + ")");
}

js> f(1, 2, 3);
I don't understand a(1, 2, 3)
js> o2.a(1, 2, 3)
12: TypeError: o2.a is not a function
js>

I'm sure I'm missing something stupid, though.
(Assignee)

Comment 42

15 years ago
Scott, I don't believe you've understood the patch :-).

- There is no requirement to pass back a delegate.  You can return the object in
which the __resolve__ hook defined the property, either |this| or the |start|
3rd parameter (see below).

- JS is dynamic so there is no knowledge of a function call being made with the
undefined result of a property access.  Either f() fails because f is not found
in the scope chain, or o.m() fails because o.m does not exist, therefore
evalutaes to |undefined|, and the call to |undefined| fails (likewise for
o[e]()).  The idea of __resolve__ is to let any undefined reference, whether to
a function to call, a getter or getter/setter, or a plain old property, result
in computation that creates a property of the desired type and value where none
existed before.

- If we were to try to hook into the JSOP_CALL/js_Invoke code, we would have to
distinguish between calling |undefined| and calling a name not found in the
scope chain, and make the two paths (which are quite different) call __DNU__. Or
would you want only the o.m() case to call __DNU__ when m does not exist (or has
|undefined| value, and exists!  Note the ambiguity), and not in the f() case
where f is not found in the scope chain?

I realize this is different from SmallTalk's doesNotUnderstand:, but JS is not
SmallTalk :-).  Anyway, here's a revised version of the example __resolve__
function:

Object.prototype.__resolve = function (id, flags, start) {
  print('Resolving ' + id + ' with flags ' + flags);
  if (flags & Object.RESOLVE_QUALIFIED)
    print('the ' + id + ' reference is qualified by a . or [] expression');
  if (flags & Object.RESOLVE_ASSIGNING)
    print('the ' + id + ' reference is the left-hand side of an assignment');
  print('this === start => ' + (this === start));

  if (id == 'foo') {
    start[id] = 42;              // resolve the magic foo property in start
    return start;
  }

  if (id == 'bar') {
    this[id] = 43;               // resolve bar in this, not start, so it is
    return this;                 // shared among all delegating objects in the
  }                              // case where this is a prototype of start

  if (id.indexOf('baz') == 0) {
    return baz_delegate;         // /^baz.*/ delegates to bar_delegate
  }

  return null;                   // id not resolved
}

/be

Comment 43

15 years ago
Assume you had the following function:

$$doesNotUnderstand(obj, fname, arguments)
{
   // do something wild to provide the missing functionality ;)
   return "magic";
};

Also assume that, without __resolve__, you hooked that into the system by
creating anonymous stubs in various slots where a full implementation of 
the method in question doesn't exist. What I'll call a "DNU wrapper" as in:

Object.prototype.foofy = function()
{
   return $$doesNotUnderstand(this, arguments.callee.getName(), arguments);
};


Given that, am I correct in thinking that I could do the following with the patch?


Object.prototype.__resolve__ = function(id, flags, start)
{
    // build the "DNU wrapper" on the fly...
    this[id] = function()
    {
       return $$doesNotUnderstand(this, id, arguments);  
    };

    return this;
};


???

Comment 44

15 years ago
Sorry, left out a question...

I'm assuming somewhere the flags would let me know that the slot in question was
"qualified" by () so I'd know to build a function to invoke rather than a value
to return?

(Assignee)

Comment 45

15 years ago
In reply to the last comment, no: the Object.RESOLVE_QUALIFIED flag (internally,
JSRESOLVE_QUALIFIED) tells whether the id was qualified by an explicit object
reference followed by a . (or equivalent form, possibly with a computed property
id, using []).  The engine doesn't know that a property reference is being
called, although it could in common cases.  But consider

  f()(2);

where

  function f() { return o.m; }

where

  o = {
    __resolve__: function (id) {
      if (id == 'm') {
        this[id] = function (x) {return x*x};
        return this;
      }
      return null;
    }
  }

When o resolves 'm', it cannot know that the caller of f is going to invoke m to
square 2.  This may seem like a contrived example -- in fact it is -- but it
makes the point that resolving a property id, getting a property value (or
setting!), and calling a value, are three distinct and possibly widely-separated
operations.

/be
(Assignee)

Comment 46

15 years ago
Comment on attachment 116807 [details] [diff] [review]
take 4

Given that this doesn't do what Bill and Scott want, and does create lots of
opportunities for anyone setting __resolve__ to screw up, I'm withdrawing it.

I'd like to get the scope->flags change landed, still, but without the
SCOPE_RESOLVE_MAGIC flag, of course.

A doesNotUnderstand facility tied to invocation could be done pretty easily. 
I'll try to patch that today.

/be
Attachment #116807 - Flags: superreview?(jband)
Attachment #116807 - Flags: review?(shaver)
(Assignee)

Updated

15 years ago
Attachment #116807 - Attachment is obsolete: true
Comment on attachment 116571 [details] [diff] [review]
just the jsdbgapi.c and jsscope.[ch] parts of attachment 116487 [details] [diff] [review]

r=shaver, with the removal of the RESOLVE_MAGIC flag.
Attachment #116571 - Flags: review?(shaver) → review+
(Reporter)

Comment 48

15 years ago
Brendan -

Sorry for the confusion and thanks for the extra effort!!

- Bill
(Assignee)

Updated

15 years ago
Target Milestone: mozilla1.4alpha → mozilla1.4beta
(Assignee)

Updated

14 years ago
Target Milestone: mozilla1.4beta → mozilla1.5alpha
(Assignee)

Updated

14 years ago
Summary: Implementation of '__resolve' function for Spidermonkey → Implementation of '__noSuchMethod__' handler for SpiderMonkey
Target Milestone: mozilla1.5alpha → mozilla1.6alpha
(Reporter)

Comment 49

14 years ago
Oooohhh....dare I hope???

As I've stated here previously, we'd *love* such a hook in SpiderMonkey :-)!

Thanks!!

- Bill
(Assignee)

Comment 50

14 years ago
Created attachment 133805 [details] [diff] [review]
ok, took a bit more doing than I implied

Should I bum's-rush this into 1.6a?

/be
(Assignee)

Updated

14 years ago
Attachment #133805 - Flags: review?(shaver)
(Assignee)

Comment 51

14 years ago
I checked in a patch similar to attachment 133805 [details] [diff] [review], see bonsai.  Testsuite
passes, no harm comes to anyone who does not set __noSuchMethod__ handlers, and
those work as far as I can tell.  Shell example input:

o = {
  __noSuchMethod__: function (id, args) {print(id, '('+args.join(', ')+')'); }
};
o.foo(1,2,3);
o.bar(4,5);
o.baz();

Output:

foo (1, 2, 3)
bar (4, 5)
baz ()

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED

Comment 52

14 years ago
Brendan,

First let me say thanks a million for this patch.

I have a couple of test cases/questions for you. Does the patch support
installation of the noSuchMethod handler on the prototypes of the native types
including Object.prototype? That would be a requirement for it to be a solid
alternative to what we currently use. 

Also, I'm assuming that within the handler 'this' refers to the targeted object
so that placing the handler on Object.prototype should imply the handler could
do the following:

// assume this function exists
function handleMissingMethod(targetObj, methodName, args, context) {...};

Object.prototype.__noSuchMethod__ = function(id, args) 
{
   return handleMissingMethod(this, id, args, arguments);
};


ss
(Reporter)

Comment 53

14 years ago
I downloaded 1.6 Alpha and wrote a quick set of tests that exercise Scott's
requirements.

It works perfectly. I've attached the test.

Thanks a lot Brendan!
(Reporter)

Comment 54

14 years ago
Created attachment 134673 [details]
Test showing fix meets all of requester criteria ;-)

Updated

14 years ago
Attachment #116487 - Flags: review?(shaver)

Comment 55

14 years ago
This caused bug 224956.
Attachment #133805 - Flags: review?(shaver) → review-
Attachment #133805 - Flags: review-

Comment 56

13 years ago
js/tests/js1_5/Object/no-such-method.js checked in.

Updated

12 years ago
Flags: testcase+

Comment 57

11 years ago
verified fixed 20060818
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.