Improve docs for std.typecons.scoped
#3016
Conversation
ntrel
force-pushed
the
scoped-docs
branch
2 times, most recently
from
b477325 to
446d330
Compare
| The class's destructor will be called when the result of `scoped()` is | ||
| itself destroyed. | ||
|
|
||
| Class instances can be embedded in a `class` or `struct` as member |
There was a problem hiding this comment.
This sentence feels like a random fact. I see two points here: (a) With scoped class instance can be placed like struct instances and (b) the type typeof(scoped!Class(args)) has to be used to use a scoped class as a member variable. The second point is actual information (I did not know that before). Maybe you can make this point more pronounced. In addition a one-sentence explanation would be nice.
There was a problem hiding this comment.
Please suggest any additional wording if needed.
|
ping |
| @@ -5141,7 +5141,16 @@ therefore avoiding the overhead of $(D new). This facility is unsafe; | |||
| it is the responsibility of the user to not escape a reference to the | |||
| object outside the scope. | |||
|
|
|||
| Note: it's illegal to move a class reference even if you are sure there | |||
| The class's destructor will be called when the result of `scoped()` is | |||
There was a problem hiding this comment.
possessive for an 's' ending word is just a trailing apostrophe: class' destructor
|
Other than the nitpick, looks good. |
| scoped. See below for an example. | ||
|
|
||
| Note: | ||
| It's illegal to move a class reference even if you are sure there |
There was a problem hiding this comment.
What does it mean to "move a class reference"? Assign it to another class reference? Call std.algorithm.mutation.move on it?
There was a problem hiding this comment.
Yes, I didn't like this language either, but I didn't know how to improve it, so I left it as-is.
There was a problem hiding this comment.
But I actually don't understand what it means. What does it mean "it's illegal to move a class reference"? What does "move" mean in this context?
There was a problem hiding this comment.
I'm not sure, I didn't write it.
There was a problem hiding this comment.
Alright, let's leave this to a future PR. Better to have a partially-better doc than to have no improvements at all.
There was a problem hiding this comment.
It's a poorly written comment. Should be "It's illegal to move an object's data". It's perfectly legal to move a pointer/reference.
There was a problem hiding this comment.
I still don't quite understand exactly what that means, though. How do you "move an object's data" if said object is being held by Scoped? Copy it elsewhere and invalidate the scoped reference? Manually overwrite the scoped reference pointer? I'm not seeing the point of that comment. Obviously, I'm missing something obvious, but that just shows how badly written that comment is? :-P
Show automatic destruction of `scoped` result. Show how `scoped` can lead to invalid class references. Show scoped member variable initialization. Move alias example before *Restrictions* section, which uses alias.
|
Added newlines, |
|
ping |
|
LGTM |
|
Auto-merge toggled on |
|
Auto-merge toggled off |
|
Fails auto-tester somewhere deeper in std.typcons, memory corruption? |
|
ping @ntrel |
| alias makeScopedA = scoped!A; | ||
| auto a3 = makeScopedA(); | ||
| auto a4 = makeScopedA(1); | ||
|
|
||
| // Restrictions | ||
| static assert(!is(typeof({ |
There was a problem hiding this comment.
This assert is failing for some reason. (Tested on git HEAD, I had to rebase on Phobos master to get it to compile, but make -f posix.mak unittest aborts with an assertion failure on this line.)
There was a problem hiding this comment.
I'm guessing something broke with scoped, and it now compiles with std.algorithm.move even though it should not.
There was a problem hiding this comment.
I think move probably always did work with scoped. In my updated #4129, I changed the assert to version(Bug) instead.
|
Any plans to revive this @ntrel ? |
|
No plans really, I've not been doing any development for a while. Thanks for reviews though. |
Then I'm going to close it, feel free to create a new pull. |
Show automatic destruction of
scopedresult.Show how
scopedcan lead to invalid class references.Show scoped member variable initialization.
Move alias example before Restrictions section, which uses alias.