-
-
Notifications
You must be signed in to change notification settings - Fork 752
Allow using Unique with interfaces #4764
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
Changes from all commits
9ea653e
b6c2e38
894bdfe
7cbea74
70a9e2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,16 +61,17 @@ $(D T) to deallocate or clean up any non-GC resources. | |
| If it is desirable to persist a $(D Unique!T) outside of its original | ||
| scope, then it can be transferred. The transfer can be explicit, by | ||
| calling $(D release), or implicit, when returning Unique from a | ||
| function. The resource $(D T) can be a polymorphic class object, in | ||
| which case Unique behaves polymorphically too. | ||
| function. The resource $(D T) can be a polymorphic class object or | ||
| instance of an interface, in which case Unique behaves polymorphically | ||
| too. | ||
|
|
||
| If $(D T) is a value type, then $(D Unique!T) will be implemented | ||
| as a reference to a $(D T). | ||
| */ | ||
| struct Unique(T) | ||
| { | ||
| /** Represents a reference to $(D T). Resolves to $(D T*) if $(D T) is a value type. */ | ||
| static if (is(T:Object)) | ||
| static if (is(T == class) || is(T == interface)) | ||
| alias RefT = T; | ||
| else | ||
| alias RefT = T*; | ||
|
|
@@ -280,6 +281,51 @@ private: | |
| assert(!ub2.isEmpty); | ||
| } | ||
|
|
||
| @system unittest | ||
| { | ||
| debug(Unique) writeln("Unique interface"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW: @JackStouffer has been working on getting rid of these superfluous debug statements
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good. I was going to request that they be removed but it's easy to submit a PR removing them all at once so I didn't bother.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, it seems it was common style in this module |
||
| interface Bar | ||
| { | ||
| int val() const; | ||
| } | ||
| class BarImpl : Bar | ||
| { | ||
| static int count; | ||
| this() | ||
| { | ||
| count++; | ||
| } | ||
| ~this() | ||
| { | ||
| count--; | ||
| } | ||
| int val() const { return 4; }; | ||
| } | ||
| alias UBar = Unique!Bar; | ||
| UBar g(UBar u) | ||
| { | ||
| debug(Unique) writeln("inside g"); | ||
| return u.release; | ||
| } | ||
| void consume(UBar u) | ||
| { | ||
| assert(u.val() == 4); | ||
| // Resource automatically deleted here | ||
| } | ||
| auto ub = UBar(new BarImpl); | ||
| assert(BarImpl.count == 1); | ||
| assert(!ub.isEmpty); | ||
| assert(ub.val == 4); | ||
| static assert(!__traits(compiles, {auto ub3 = g(ub);})); | ||
| debug(Unique) writeln("Calling g"); | ||
| auto ub2 = g(ub.release); | ||
| debug(Unique) writeln("Returned from g"); | ||
| assert(ub.isEmpty); | ||
| assert(!ub2.isEmpty); | ||
| consume(ub2.release); | ||
| assert(BarImpl.count == 0); | ||
| } | ||
|
|
||
| @system unittest | ||
| { | ||
| debug(Unique) writeln("Unique struct"); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work with
extern(C++)orextern(Windows)classes & interfaces? If not we should not allow them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ZombineDev Good luck detecting them. This is something that @bbasile has been working on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know about
extern(Windows)- isn't that similar toextern(C)which wouldn't work with classes (I really haven't done much with Windows and D, and Windows linking is way more complicated in general than *nix)? However, as for something likeextern(C++)or COM interfaces, wouldn't checkingdo the trick?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is, if the idea is to exclude them anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmdavis the problem is that
Objectis a class, so ifTis an interface it won't extendObject.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know a lot about
extern(c++)orextern(Windows), but from reading http://dlang.org/spec/interfaceToC.html, it seems likeextern(Windows)isn't really relevant since it is more about function calling conventions, and from http://dlang.org/spec/cpp_interface.html it seems like this might work with C++ classes. Although it isn't really clear if it is safe to calldestroyon a C++ class. Of course, Unique certainly won't clean up the memory for a C++ class, but that is true regardless of ifRefTisTorT*There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO this shouldn't be stalled on whether it works with extern(C++)/extern(Windows) classes & interfaces. If they didn't work before then this PR doesn't change the status quo.