Skip to content

Conversation

@WalterBright
Copy link
Member

…meter is not a pointer

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
18963 enhancement Relax restrictions on 'return' parameters when parameter is not a pointer

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8346"

@WalterBright WalterBright added Review:Blocking Other Work review and pulling should be a priority Review:Vision Vision Plan https://wiki.dlang.org/Vision/2018H1 labels Jun 9, 2018
@WalterBright
Copy link
Member Author

This is blocking dlang/phobos#6560

@JinShil JinShil added the Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org label Jun 9, 2018
@WalterBright
Copy link
Member Author

@JinShil the spec doesn't mention return parameters at all.

@JinShil
Copy link
Contributor

JinShil commented Jun 10, 2018

the spec doesn't mention return parameters at all.

It mentions return ref parameters. But the lack of documentation is all the more reason to document it now, and through that documentation justify this PR.

@schveiguy
Copy link
Member

Just want to voice my support here. I don't have enough experience to review this, to know that it's correctly implemented. But speaking from experience with inout (I originally wanted inout to have similar errors like "can't have inout return without inout parameters" and it's horrible for generic code).

@WalterBright
Copy link
Member Author

WalterBright commented Jun 11, 2018

But the lack of documentation is all the more reason to document it now, and through that documentation justify this PR.

  1. Dip1000 is not in the spec. It certainly needs to be done, but I'm pretty reluctant to put the entire Phobos implementation on hold for another year because of this. Making everything serial instead of parallel is not productive.

  2. I can't get even simple PRs to the spec pulled. Last January, I resolved to go through and make the spec more accurate and more formal. But this all ground to a halt due to complete disinterest in anyone reviewing or pulling them.

  3. This PR is a trivial and obvious improvement. The proposed behavior is the same as for scope which is already implemented.

  4. This PR is not a dot i and cross t thing. It's blocking valuable and important work.

  5. There's an Enhancement Request referenced which describes it.

@JinShil
Copy link
Contributor

JinShil commented Jun 11, 2018

Dip1000 is not in the spec. It certainly needs to be done, but I'm pretty reluctant to put the entire Phobos implementation on hold for another year because of this. Making everything serial instead of parallel is not productive.

Too often, features are added to D without sufficient documentation, and DIP1000 is the worst offender. I'm not asking that you remedy that entire problem right now, I'd just like to see a section on Return Parameters with a brief explanation of it's purpose, a bullet point calling out this PR's exception, and perhaps an example. I don't think that's too much to ask.

I can't get even simple PRs to the spec pulled. Last January, I resolved to go through and make the spec more accurate and more formal. But this all ground to a halt due to complete disinterest in anyone reviewing or pulling them.

Indeed, and I'm sorry about that. There's a lot of disinterest in the things that I and others care about too. But it's not just disinterest. Many of us are not capable of certifying certain PRs. I recommend you put a time limit on those PRs that aren't getting attention. If noone raises any objections after a week, merge it. Noone has a right to complain if they chose not to participate, and we can always submit corrections if errors are found after the fact.

This PR is not a dot i and cross t thing. It's blocking valuable and important work.

The sooner you add the docs, the sooner you'll be unblocked.

There's an Enhancement Request referenced which describes it.

Bugzilla is not a substitute for proper documentation.

@andralex
Copy link
Member

Please don't let bureaucracy smother progress. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merge:auto-merge Review:Blocking Other Work review and pulling should be a priority Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org Review:Vision Vision Plan https://wiki.dlang.org/Vision/2018H1 Severity:Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants