Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

How to compare if two functions are equal. #144

Closed
floitschG opened this Issue · 19 comments

7 participants

@floitschG
Collaborator

void foo() {}
void main(){
  print(foo == foo); // => false on the VM.
}

We need to specify how references to (static) functions are handled.
This includes the following cases:
 o.foo == o.foo
 o.foo === o.foo
 foo == foo // for a static function foo.
 foo === foo

@gbracha
Collaborator

Set owner to @gbracha.
Added Accepted label.

@gbracha
Collaborator

This amounts to deciding whether closurizing produces canonicalized closures or not. For static functions, there might be a special case as we may wish to start treating them as compile time constants.

@DartBot
Collaborator

This comment was originally written by fwebstud...@gmail.com


htmlelement.on.eventtype.remove not work, solutions like observable not work, this is very important to fix this

@rakudrama
Collaborator

Canonicalization of property extraction is very useful, whether or not other closures are canonicalized.
Consider:

#­1 What the developer wants to write (fwebstudio is expecting something like this to work)

class Widget {
  handleClick() { ... send data ... }

  HTMLElement element;
  Widget(this.element);

  enter() {
    element.on.click.add(handleClick);
  }

  exit() {
    element.on.click.remove(handleClick);
  }
}

#­2 Attempt to canonicalize the property extraction.

class Widget {
  handleClick() { ... send data ... }

  HTMLElement element;
  var clickHandler = handleClick; // Error, not a constant, though harmless because 'this' is not dereferenced.
  Widget(this.element);

  enter() {
    element.on.click.add(clickHandler);
  }

  exit() {
    element.on.click.remove(clickHandler);
  }
}

#­3 Second attempt to canonicalize the property extraction

class Widget {
  handleClick() { ... send data ... }

  HTMLElement element;
  var clickHandler;
  Widget(this.element) {
    clickHandler = handleClick;
  }

  enter() {
    element.on.click.add(clickHandler);
  }

  exit() {
    element.on.click.remove(clickHandler);
  }
}

#­4 Third attempt to canonicalize the property extraction - lazy closure allocation version.

class Widget {
  handleClick() { ... send data ... }

  HTMLElement element;
  var _clickHandler;
  Widget(this.element);

  get clickHandler() {
    if (_clickHandler == null) _clickHandler = handleClick;
    return _clickHandler;
  }

  enter() {
    element.on.click.add(clickHandler);
  }

  exit() {
    element.on.click.remove(clickHandler);
  }
}

It would be trivial to make Frog generate JS code for #­1 that behaves equivalent to #­4.

I have programmed applications using the Google Closure library http://code.google.com/closure/library/ .
With Closure, where you have to write JavaScript code like #­3 or #­4 (... = handleClick; becomes ... = goog.bind(this.handleClick, this); )
It is an unbelievably tedious task.

@munificent
Collaborator

+1.

Yes please make this work. Event handlers are unpleasant to work with without being able to unregister them by identity and canonicalization is required for that.

When I was working on the View class I spent a bunch of time trying to awkwardly work around this deficiency. It would be really helpful if this just worked (as it does in C#).

@DartBot
Collaborator

This comment was originally written by @sethladd


This came up again: http://japhr.blogspot.com/2012/03/really-really-removing-event-handlers.html

@DartBot
Collaborator

This comment was originally written by mikeebe...@gmail.com


This bug (or unusual behavior) cost me a bit of time; I thought my event-handler logic was messed up when trying to remove an event handler I had previously added, but it was due to this issue. The workaround mentioned in the blog posting in comment#­6 does work.

Essentially, by creating a variable that references our event-handler method, and providing that VARIABLE to the handler parameter in EventListenerList add/remove operations (vs. directly referencing the desired method), Dart can accurately determine "equality" for removing a previous added event. Though this approach works for now, it seems such a workaround should be unnecessary once Dart matures a wee bit more.

@anders-sandholm

Added apr30-triage label.

@anders-sandholm

Removed apr30-triage label.

@anders-sandholm

Added triage1 label.

@anders-sandholm

Added this to the Later milestone.
Removed triage1 label.

@anders-sandholm

Issue #167 has been merged into this issue.

@anders-sandholm

Removed this from the Later milestone.
Added this to the M1 milestone.

@sethladd
Owner

This just came up in the hackathon.

@gbracha
Collaborator

Issue #3100 has been merged into this issue.

@DartBot
Collaborator

This comment was originally written by @tomyeh


Issue #167 was merged into this issue. I assume it implies the hashCode method will be implemented too (such that a function can be put into a map or set).

@gbracha
Collaborator

This issue has been carefully considered and we decided it won't fly.
Canonicalizing these can be done in several ways, none of which are attractive. We could keep per-isolate hash table keyed by objects. Every object that had a closurization applied to it would be a key, and then a secondary hash would look up based on the property name. This a space leak (weak pointers are a problem especially on JS) and dead slow.

We coud, as suggested, add a field to any class whose instances are ever accessed this way. Except that it isn't as easy as it seems. What if the access is outside the class? Are we to require a global analysis (which, without static typing, is actually non-trivial). So scratch that.

In short canonicalization is not an option, unless you want things to be bloated and or slow.

We also considered an == method for closures, but there are very good reasons why no one ever does anything but identity based equality for closures. In general, you cannot tell if two functions are equal.

We considered other options as well, but they all lead to complexity or flakiness of one kind or another. What you really need to do is the code in version (3) of class Widget above, which is not all that difficult.


Removed this from the M1 milestone.
Added WontFix label.

@floitschG
Collaborator

Sevaral notes and questions:

  • Does this also apply for static functions? Nothing in your reply looks like an obstacle for static functions. In fact, dart2js does canonicalization for static functions without any problems.
  • there is no need for weak hashtables in JS. Afaics canonicalization in JS should be feasible. We could either dynamically replace the getter, or use add an additional field (we already do the global analysis you are talking about).
  • Are we allowed to canonicalize? The VM and the dart->js compiler currently have different behavior.
@gbracha gbracha was assigned by floitschG
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.