Skip to content

Conversation

@WalterBright
Copy link
Member

See enhancement request for details.

To keep this PR from being too complex, I didn't do return inference. That'll be on a follow-on later.

This will be necessary to get Phobos to compile with -dip1000. Hence the Vision label.

@WalterBright WalterBright added the Review:Vision Vision Plan https://wiki.dlang.org/Vision/2018H1 label Jul 19, 2018
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
19097 enhancement Extend Return Scope Semantics

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#8504"

@WalterBright WalterBright force-pushed the fix19097 branch 7 times, most recently from cc1088b to 89d4a2f Compare July 19, 2018 09:46
@wilzbach
Copy link
Contributor

auto-tester failures are related to the Phobos modules we check with -dip1000.
Not sure about a good solution, but dlang/phobos#6638 would help for now (and unblock this PR).

@WalterBright
Copy link
Member Author

Curiously, the autotester was green last night.

@wilzbach
Copy link
Contributor

Yes, we re-enabled -dip1000 testing for the auto-tester today (at least for all the modules which are part of the dip1000.mak).

@JinShil
Copy link
Contributor

JinShil commented Aug 18, 2018

I can't understanding anything in this PR because there is no spec or other documentation describing the semantics.

@WalterBright
Copy link
Member Author

no documentation

https://issues.dlang.org/show_bug.cgi?id=19097

@JinShil
Copy link
Contributor

JinShil commented Aug 18, 2018

Not good enough. The appropriate place for documenting the semantics of the language is https://github.com/dlang/dlang.org/tree/master/spec

@WalterBright
Copy link
Member Author

It has all that's needed to understand this PR. If you have any other questions about how/why, please post here or (better) on the bugzilla page.

@JinShil
Copy link
Contributor

JinShil commented Aug 18, 2018

There needs to be a spec update accompanying each of these DIP1000-related PRs (e.g #8408, #8346), not only for the review process, but also so users know how to leverage these features. Furthermore, these features cannot be evaluated in isolation; they need to be evaluated in the context of the existing language, and all of the prior PRs that were merged without any accompanying documentation, so the hole is just getting deeper.

Users are only going to encounter confusion and uncertainty without these semantics documented in the spec. They can't even file bugs with any confidence, because they don't have a spec to compare their symptoms with and instead resort to "Is this a bug?", "What's the status of @safe/DIP1000?" posts on the forum. Even when attempting to answer such questions, veterans of the language are just resorting to educated guesses.

@WalterBright WalterBright added the Review:Blocking Other Work review and pulling should be a priority label Aug 21, 2018
@WalterBright WalterBright force-pushed the fix19097 branch 2 times, most recently from 79b9fc9 to aaf8454 Compare October 26, 2018 09:45
@thewilsonator thewilsonator added Review:Needs Changelog A changelog entry needs to be added to /changelog Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org labels Nov 4, 2018
@thewilsonator thewilsonator removed Review:Needs Changelog A changelog entry needs to be added to /changelog Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org labels Dec 2, 2018
@thewilsonator
Copy link
Contributor

thewilsonator commented Jan 5, 2019

This is now documented in dlang/dlang.org#2453, I'm still very disappointed with the way this has been handled.

@thewilsonator
Copy link
Contributor

Auto-merge toggled on

@thewilsonator thewilsonator merged commit 4719804 into dlang:master Jan 5, 2019
@WalterBright WalterBright deleted the fix19097 branch January 5, 2019 18:07
@JinShil
Copy link
Contributor

JinShil commented Jan 9, 2019

FWIW, I think this was merged prematurely. My objection wasn't just about the documentation. We needed the documentation in order to properly review this PR. After spending some time studying what @WalterBright was proposing, I offered an alternative in https://issues.dlang.org/show_bug.cgi?id=19097 so users don't have to rely on convention to get the result, but my suggestion was never acknowledged or addressed. Others brought up issues on the forum, but they also were never adequately addressed.

I suppose it's all moot now, but this can serve as a good example of what not to do in the future. I fear that having to infer return based on convention is not going to end well for D in the long run.

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

Labels

Review:Blocking Other Work review and pulling should be a priority 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