Skip to content

Allow alias this to use assignment-style syntax#15122

Merged
dlang-bot merged 2 commits intodlang:masterfrom
pbackus:alias-this-syntax
May 10, 2023
Merged

Allow alias this to use assignment-style syntax#15122
dlang-bot merged 2 commits intodlang:masterfrom
pbackus:alias-this-syntax

Conversation

@pbackus
Copy link
Contributor

@pbackus pbackus commented Apr 23, 2023

When assignment-style syntax for alias declarations was first implemented in DMD 2.061 [1][2], alias this = identifier; was accepted as equivalent to the existing alias identifier this;. One release later, in DMD 2.062, it was removed. [3]

The rationale for this change, given in both the changelog [4] and a related spec PR thread [5], was to allow for the possibility that, in the future, the syntax alias this = super.this; might be used to merge a derived class's constructor overload set with that of its base class. However, this proposal was never implemented, and seems to have been abandoned in the intervening years.

For the sake of consistency, and since the rationale for its removal no longer applies, this commit reinstates alias this = identifier; as valid syntax for an alias this declaration.

[1] #1187
[2] https://dlang.org/changelog/2.061.html
[3] #1413
[4] https://dlang.org/changelog/2.062.html
[5] dlang/dlang.org#200 (comment)

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @pbackus! 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 coverage diff by visiting the details link of the codecov check)
  • 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

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

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

dub run digger -- build "master + dmd#15122"

pbackus added a commit to pbackus/dlang.org that referenced this pull request Apr 23, 2023
@pbackus
Copy link
Contributor Author

pbackus commented Apr 23, 2023

Spec PR: dlang/dlang.org#3589

@pbackus
Copy link
Contributor Author

pbackus commented Apr 23, 2023

CI failure on macOS is issue 23846, currently affecting all DMD PRs.

@pbackus
Copy link
Contributor Author

pbackus commented Apr 23, 2023

Added changelog.

@dkorpel dkorpel removed the Review:Needs Changelog A changelog entry needs to be added to /changelog label Apr 23, 2023
@PetarKirov
Copy link
Member

Thank you @pbackus for tackling this issue! Reading the old discussions, I get the impression that alias X this syntax has been bothering almost everyone (I'm glad it wasn't just me 😄), so I know many people would be happy to see it be replaced with something better fitting the general design of the language 👍

What do you all think think about the two alternatives:

@pbackus
Copy link
Contributor Author

pbackus commented Apr 23, 2023

@PetarKirov This is an addition, not a replacement. As far as I know there are no plans to get rid of alias X this. And given that alias X this isn't going anywhere, consistency demands that we also allow alias this = X.

@PetarKirov
Copy link
Member

As far as I know there are no plans to get rid of alias X this.

Most likely, that's only because #1413 was merged and we dropped the ball on #1341. If the case was reversed, it's likely that that if the old alias Old New; syntax was deprecated (which I thought was, but looking at here it seems it's only forbidden by the D style CI checks), alias this would follow suit. Anyhow, that's a future topic for discussion.

And given that alias X this isn't going anywhere, consistency demands that we also allow alias this = X.

I don't follow. As I mentioned before, "alias this" is about sub typing, so the alias this : X syntax would be more consistent with is(T : X). alias this = x would imply that each time you're accessing this.y inside the aggregate, you're always accessing this.x.y, which is not exactly the case (e.g. if y is defined directly in the aggregate and not inside its x member).

@PetarKirov
Copy link
Member

PetarKirov commented Apr 24, 2023

@pbackus
Copy link
Contributor Author

pbackus commented Apr 24, 2023

Anyhow, that's a future topic for discussion.

Yes. Any redesign or replacement for alias this will need a DIP and a long deprecation period, at the very least, all of which is out-of-scope for this PR thread.

alias this = x would imply that each time you're accessing this.y inside the aggregate, you're always accessing this.x.y, which is not exactly the case (e.g. if y is defined directly in the aggregate and not inside its x member).

This argument applies equally well to both alias this = x and alias x this.

Again, this PR is not a referendum on alias this in general; it is about removing an unnecessary special case from D's syntax.

@dkorpel
Copy link
Contributor

dkorpel commented May 2, 2023

Atila approved this in an email

@WalterBright
Copy link
Member

Would like to see a spec PR for this.

@pbackus
Copy link
Contributor Author

pbackus commented May 10, 2023

@WalterBright Spec PR is dlang/dlang.org#3589, as previously mentioned.

@WalterBright
Copy link
Member

good

pbackus added 2 commits May 10, 2023 11:43
When assignment-style syntax for alias declarations was first
implemented in DMD 2.061 [1][2], `alias this = identifier;` was accepted
as equivalent to the existing `alias identifier this;`. One release
later, in DMD 2.062, it was removed. [3]

The rationale for this change, given in both the changelog [4] and a
related spec PR thread [5], was to allow for the possibility that, in
the future, the syntax `alias this = super.this;` might be used to merge
a derived class's constructor overload set with that of its base class.
However, this proposal was never implemented, and seems to have been
abandoned in the intervening years.

For the sake of consistency, and since the rationale for its removal no
longer applies, this commit reinstates `alias this = identifier;` as
valid syntax for an `alias this` declaration.

[1] dlang#1187
[2] https://dlang.org/changelog/2.061.html
[3] dlang#1413
[4] https://dlang.org/changelog/2.062.html
[5] dlang/dlang.org#200 (comment)
@dlang-bot dlang-bot merged commit b4ea5d1 into dlang:master May 10, 2023
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.

5 participants