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

Implement DIP36 - 'scope ref' and 'in ref' for proper rvalue reference #1903

Closed
wants to merge 1 commit into from

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Apr 17, 2013

http://wiki.dlang.org/DIP36

scope ref and in ref parameters can bind both lvalues and rvalues.

@Dgame
Copy link

Dgame commented Apr 17, 2013

Good work Kenji, as always. Hope it will be merged soon. :)

@TurkeyMan
Copy link
Contributor

Oh... my... god...!
This is it, the patch I've been waiting for all these years!!!

@donc
Copy link
Collaborator

donc commented Apr 18, 2013

The simplicity of this patch gives me confidence that it is good language design.

@TurkeyMan
Copy link
Contributor

It helps that the feature makes perfect sense. When I learned that scope existed, I presumed this is what it was for. It seems like it was destined for this purpose from the start.

@Dgame
Copy link

Dgame commented Apr 18, 2013

Let us hope that Walter and Andrei think the same about it.

@mihails-strasuns
Copy link

Him, it does not really implement "prohibit escaping of scope" right now, does it?

@9rnsr
Copy link
Contributor Author

9rnsr commented Apr 19, 2013

it does not really implement "prohibit escaping of scope" right now,

No. But I think it should be done near the future.

@mihails-strasuns
Copy link

Sure. It is just important to not forget this is rather unsafe and dangerous feature until compiler control comes.

@TurkeyMan
Copy link
Contributor

@Dicebot People already do this by force of habit:

void func(ref const matrix m);

func(x.getMatrix()); // error: @#%^&

matrix temp = x.getMatrix();
func(temp);

Even if scope doesn't 'work' in the mean time, it at least gives the programmer the guide, so they know they can/should pass a temp if they want to. Right now, they can't possibly know if it's safely able to receive a temp or not, and just do it anyway because it usually is safe.

@mihails-strasuns
Copy link

@TurkeyMan I know. Right now it is never reliably safe which is a big hole in the type system. Introducing solution that pretends to give guarantees but actually does not may make it even worse. I'd prefer to see it together with scope implemented.

@TurkeyMan
Copy link
Contributor

I just can't agree with that. This is a better situation whether scope is fully implemented or not. It will change the probability of someone passing a temp to a function where it's not safe to do so.
Right now, the recommended behaviour is "subvert the safety of your code, and at the cost of convenience and code readability". That is a truly unacceptable state to be in. With this change, the recommendation will become "don't subvert the type system" as one expects.
Sure, having scope fully implemented would be great! Who's going to do it? Taking steps in the right direction is better than waiting forever.

@Dgame
Copy link

Dgame commented Apr 23, 2013

You can close this pull. It was so far rejected by Andrei.

@andralex
Copy link
Member

@9rnsr it would be great if you participated in the discussion, in particular how big the impact would be if we made ref safe.

@9rnsr
Copy link
Contributor Author

9rnsr commented Apr 24, 2013

@andralex I replied in the forum thread (http://forum.dlang.org/post/mailman.907.1366764128.4724.digitalmars-d@puremagic.com). I think that removing "parameterized-ref" (== current auto ref) from language is not good.

@w0rp
Copy link

w0rp commented Aug 28, 2013

I'd love to see something like this get merged soon. I'd like to cut down on my extra "just for rvalue support" overloads.

@ghost
Copy link

ghost commented Aug 28, 2013

out ref isn't related to this pull, however it should be allowed, there's no reason not to. This is issue Issue 8121.

@ghost
Copy link

ghost commented Aug 28, 2013

@9rnsr: Since your pull might sit here for a long while, would it be ok if I made a fix for Issue 8121.

It seems all I have to do is remove the parser limitation for scope ref and scope out, however I'm not sure whether any other code has to be changed after this parser limitation removal?

@mihails-strasuns
Copy link

@AndrejMitrovic I'd say fixing anything related to scope until it is actually defined and implemented is road to more troubles in future with breaking the code if it is ever going to be implemented.

@ghost
Copy link

ghost commented Aug 29, 2013

The code that would break is code which does what scope is technically already supposed to prevent, e.g.:

int* p;
void foo(scope int* x) { p = x; }
void bar(scope ref int x) { p = &x; }

This code would break once scope is properly implemented, but that's a good thing. I think this is about the only definition of scope in a parameter list that I know of.

@mihails-strasuns
Copy link

@AndrejMitrovic
When I asked @andralex has said several time that currently there is no such thing a "properly implemented scope" design and it should be considered completely new feature. Currently scope is no-op for everything but scope delegates and there are no hard promises how it is going to look like so adding more compiling cases with scope pretty much equals to more code breakage once it is implemented.

@ghost
Copy link

ghost commented Aug 29, 2013

@Dicebot:
I see what you mean. Well I guess we have to wait for someone to write a DIP on it.

@Dgame
Copy link

Dgame commented Aug 29, 2013

We have already tried it... :D

@mihails-strasuns
Copy link

@Dgame there are no DIP's on scope definition that I am aware of.

@Dgame
Copy link

Dgame commented Aug 29, 2013

@Dicebot It was meant as a joke, because we also had conceived functionality for scope with our scope/in ref proposal.

@MetaLang
Copy link
Member

This looks like it should be closed in light of the recent action around DIP25 and DIP69 @9rnsr @andralex.

@9rnsr 9rnsr closed this Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants