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

Fixes issues 16271, 18825, 8065, 5050 - Allow function literals return by reference #9558

Merged
merged 4 commits into from
Apr 7, 2019

Conversation

BorisCarvajal
Copy link
Member

@BorisCarvajal BorisCarvajal commented Apr 4, 2019

respective change in spec dlang/dlang.org#2618

@dlang-bot
Copy link
Contributor

dlang-bot commented Apr 4, 2019

Thanks for your pull request and interest in making D better, @BorisCarvajal! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
5050 normal No way to declare delegates with ref return
8065 enhancement No way to write function/delegate literals returning ref T
16271 enhancement Should be able to express that a lambda returns by reference
18825 enhancement No syntax for function literal returning a reference

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

@BorisCarvajal BorisCarvajal changed the title Fix issues 16271, 18825, 8065, 5050 - Allow function literals return by reference Fixes issues 16271, 18825, 8065, 5050 - Allow function literals return by reference Apr 5, 2019
@thewilsonator
Copy link
Contributor

Please take a look at the buildkite ocean failures

./src/ocean/transition.d(42): Error: basic type expected, not `static`
--
  | ./src/ocean/transition.d(42): Error: found `static` when expecting `)`
  | ./src/ocean/transition.d(42): Error: semicolon expected following auto declaration, not `immutable`
  | ./src/ocean/transition.d(42): Error: declaration expected, not `)`
  | ./src/ocean/meta/types/Qualifiers.d(73): Error: basic type expected, not `static`
  | ./src/ocean/meta/types/Qualifiers.d(73): Error: found `static` when expecting `)`
  | ./src/ocean/meta/types/Qualifiers.d(73): Error: found `immutable` when expecting `)`
  | ./src/ocean/meta/types/Qualifiers.d(73): Error: found `)` when expecting `;`
  | ./src/ocean/meta/types/Qualifiers.d(73): Error: found `)` instead of statement
  | ./src/ocean/meta/types/Qualifiers.d(169): Error: basic type expected, not `static`
  | ./src/ocean/meta/types/Qualifiers.d(169): Error: found `static` when expecting `)`
  | ./src/ocean/meta/types/Qualifiers.d(169): Error: found `immutable` when expecting `)`
  | ./src/ocean/meta/types/Qualifiers.d(169): Error: no identifier for declarator `U`
  | ./src/ocean/meta/types/Qualifiers.d(169): Error: declaration expected, not `)`
  | ./src/ocean/meta/types/Qualifiers.d(173): Error: declaration expected, not `else`
  | ./src/ocean/meta/types/Qualifiers.d(176): Error: unrecognized declaration
  | ./src/ocean/core/Test.d(230): Error: semicolon expected following function declaration
  | ./src/ocean/core/Test.d(232): Error: declaration expected, not `if`

@JinShil
Copy link
Contributor

JinShil commented Apr 5, 2019

Please take a look at the buildkite ocean failures

This may have been caused by Ocean's recent 5.0.0 release.

@BorisCarvajal
Copy link
Member Author

I checked and the error is the same with stable.

@PetarKirov
Copy link
Member

Thanks for working on this @BorisCarvajal!

@RazvanN7
Copy link
Contributor

RazvanN7 commented Apr 5, 2019

Could you please add the text "Fix Issue" followed by the issue numbers in a commit so that the dlang bot automatically links the issues with this PR?

@thewilsonator
Copy link
Contributor

Specifically the commit message should contain "Fixes issues 16271, 18825, 8065, 5050", i.e. make it the same as the PR title.

@BorisCarvajal
Copy link
Member Author

done

@jacob-carlborg
Copy link
Contributor

  • It looks like all examples are using =>. Perhaps add some examples with using curly braces.
  • What about declaring a variable of a delegate type that returns ref, is that already possible?

@BorisCarvajal
Copy link
Member Author

BorisCarvajal commented Apr 6, 2019

  • I just tested the refness of the returns, the syntax is fairly proved already.
    I was going to put some checks with those but I thought was overkill, I'll add a few anyways.
  • Yes, you can do:
int x;
ref int dg() { return x; }
auto d = &dg; // or alias d = dg;
d() = 2;

src/dmd/parse.d Outdated Show resolved Hide resolved
@PetarKirov PetarKirov added the Merge:72h no objection -> merge The PR will be merged if there are no objections raised. label Apr 6, 2019
src/dmd/parse.d Outdated Show resolved Hide resolved
@jacob-carlborg
Copy link
Contributor

Make sure the commits are squashed when merging.

@thewilsonator
Copy link
Contributor

@ZombineDev I had the spec PR on 72h -> merge, I don't think we need to wait for both.

@thewilsonator thewilsonator added Merge:auto-merge-squash and removed Merge:72h no objection -> merge The PR will be merged if there are no objections raised. labels Apr 7, 2019
@dlang-bot dlang-bot merged commit 3e229cd into dlang:master Apr 7, 2019
@wilzbach
Copy link
Member

wilzbach commented Apr 7, 2019

@BorisCarvajal as this is definitely a new feature that DMD users should be made aware of (e.g. so that they can update their D grammar/parsers), it would be great if you could follow up with a small changelog entry:

https://github.com/dlang/dmd/tree/master/changelog

Thanks!

This also will help to expose your work to the wider D community.

@andralex
Copy link
Member

andralex commented Apr 8, 2019

Great work. Thanks @BorisCarvajal !

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

Successfully merging this pull request may close these issues.

9 participants