Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signals fixes #567

Merged
merged 4 commits into from Jul 2, 2012
Merged

Signals fixes #567

merged 4 commits into from Jul 2, 2012

Conversation

denis-sh
Copy link
Contributor

@denis-sh denis-sh commented May 4, 2012

Looks like nobody uses signals...

Maybe, because for now it's unusable because if a class have a signal it's impossible for a derived class to have signals (Issue 8031).

With this pull it's impossible for a derived class to have signals only if a base class has the one and only signal (because of Issue 5028) and this isn't a major luck of functionality.

…erived class to have any signals

All `Signal` methods should be `final` because if they aren't it's impossible to add any signals into a derived class.

And yes, lets make `Signal` a `mixin template`.
@shoo
Copy link
Collaborator

shoo commented May 10, 2012

I had been going to use signals, too, but I judged it is unusable, and I implemented it by myself. ( https://github.com/shoo/voile/blob/eac13cac65981101ab7552d8e44cd4b479836f0e/voile/misc.d#L798 )
Its slot_t cannot be used with delegate of member function of struct/union and closures, this is fatal...
Is it really effective to make Signal a mixin template?
... I feel the need of the redesign.

@denis-sh
Copy link
Contributor Author

Its slot_t cannot be used with delegate of member function of struct/union and closures, this is fatal...

Totally agree. From std.signals docs:

BUGS: Slots can only be delegates formed from class objects or interfaces to class objects. If a delegate to something else is passed to connect(), such as a struct member function, a nested function or a COM interface, undefined behavior will result.

And this limitation in a BUGS section. We need a way to distinguish different types of delegates. @WalterBright wants them to be indistinguishable, but I believe it's a wrong way. std.signals unusable state is a proof.

I really need ways to:

  1. know if a pointer points to immutable data section (needed for fast toStringz implementation)
  2. distinguish type of a delegate

assert(!o3.i && o3.l == 33 && o3.str == "str3");

// delete observers
delete o1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use destroy (from the object module) instead? delete will be deprecated eventually, so let's not add more of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 7134b60.
Not sure if UFCS is better: a.destroy() or destroy(a)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it really matters. Looks good now, thanks!

@alexrp
Copy link
Member

alexrp commented Jun 10, 2012

Looks good other than the use of delete which isn't really related to the functionality of the pull request.

I say we merge this. Better to reject things as early as possible than allow runtime failures to happen. We can always open up the API later if we find a way to solve the aforementioned problems.

andralex added a commit that referenced this pull request Jul 2, 2012
@andralex andralex merged commit 9c7002b into dlang:master Jul 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants