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

Deprecate alias this for classes v2 #14812

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

RazvanN7
Copy link
Contributor

No description provided.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @RazvanN7! 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#14812"

@RazvanN7
Copy link
Contributor Author

cc @Geod24

@RazvanN7
Copy link
Contributor Author

cc @WalterBright

@benjones
Copy link
Contributor

Wow, the only buildkite failures are phobos!

Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changelog entry

@pbackus
Copy link
Contributor

pbackus commented Jan 14, 2023

Should this include interfaces too?

@RazvanN7
Copy link
Contributor Author

I had no idea you could use alias this in interfaces. Yes, definitely, interfaces should be included too.

@RazvanN7
Copy link
Contributor Author

@thewilsonator added changelog entry
@pbackus included interfaces

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Jan 26, 2023

The GDC tester fails with:

src/dmd/dtoh.d:294:12: error: declaration expected, not 'foreach'
  294 |     static foreach(member; __traits(allMembers, Context))
      |            ^
src/dmd/dtoh.d:294:28: error: declaration expected, not '__traits'
  294 |     static foreach(member; __traits(allMembers, Context))
      |                            ^
src/dmd/dtoh.d:3037:1: error: unrecognized declaration
 3037 | }
      | ^

when trying to build the compiler. @ibuclaw does gdc not recognize static foreach?

Later edit: It seems that gdc-10 does recognize static foreach, maybe the tester uses an older version?

@adamdruppe
Copy link
Contributor

gdc added static foreach i think in version 10. Version 9 and below do not have it.

You can make a helper function concat the strings and return them in one go for a use like this to be compatible with more ocmpailers.

@RazvanN7
Copy link
Contributor Author

gdc added static foreach i think in version 10. Version 9 and below do not have it.

Yeah, I just consulted the changelog: https://gcc.gnu.org/gcc-10/changes.html (ctrl-f "static foreach").

Is anything preventing us from upgrading the tester to gdc10?

@ibuclaw
Copy link
Member

ibuclaw commented Jan 26, 2023

gdc added static foreach i think in version 10. Version 9 and below do not have it.

Yeah, I just consulted the changelog: https://gcc.gnu.org/gcc-10/changes.html (ctrl-f "static foreach").

Is anything preventing us from upgrading the tester to gdc10?

Yes, me. Don't break gdc's bootstrap, else I'll revert.

@RazvanN7 RazvanN7 force-pushed the deprecate_alias_this_for_classes branch from bd8d9fa to 4ed4861 Compare February 14, 2023 11:54
@RazvanN7
Copy link
Contributor Author

Spec PR: dlang/dlang.org#3498

@RazvanN7 RazvanN7 closed this Feb 14, 2023
@RazvanN7 RazvanN7 reopened this Feb 14, 2023
@RazvanN7 RazvanN7 force-pushed the deprecate_alias_this_for_classes branch from 4ed4861 to 5d985e2 Compare February 15, 2023 09:47
@RazvanN7 RazvanN7 force-pushed the deprecate_alias_this_for_classes branch from 5d985e2 to 838277b Compare February 15, 2023 10:25
@RazvanN7 RazvanN7 merged commit af7817b into dlang:master Feb 15, 2023
@ibuclaw
Copy link
Member

ibuclaw commented Mar 15, 2023

There's more deprecated messages when running the testsuite:

testsuite/libphobos.hash/test_hash.d:293: warning: alias this for classes/interfaces is deprecated
testsuite/libphobos.hash/test_hash.d:300: warning: alias this for classes/interfaces is deprecated 

These excess warnings breaks the testsuite.

WalterBright added a commit to WalterBright/dmd that referenced this pull request Jun 16, 2023
atilaneves pushed a commit to atilaneves/dmd that referenced this pull request Jun 30, 2023
dkorpel pushed a commit that referenced this pull request Jul 1, 2023
dkorpel pushed a commit that referenced this pull request Jul 1, 2023
dkorpel pushed a commit that referenced this pull request Jul 1, 2023
dkorpel pushed a commit that referenced this pull request Jul 5, 2023
dkorpel pushed a commit that referenced this pull request Jul 14, 2023
dkorpel pushed a commit that referenced this pull request Jul 17, 2023
dkorpel pushed a commit that referenced this pull request Jul 17, 2023
dkorpel added a commit that referenced this pull request Jul 18, 2023
* druntime: Restrict some `pragma(inline, false)` kludges to DMD only

They appear related to DMD's inlining at the AST level; LDC at least
doesn't need them, so let the optimizer decide for non-DMD backends.

* Fix little C++ header regression

* Expose VarArg.KRvariadic to C++ headers

* Revert "Deprecate alias this for classes v2 (#14812)" (#15326)

This reverts commit af7817b.

* Add changelog for catch qualifier deprecation & update release no

---------

Co-authored-by: Martin Kinkelin <noone@nowhere.com>
Co-authored-by: Walter Bright <WalterBright@users.noreply.github.com>
Co-authored-by: Nick Treleaven <ntrel002@gmail.com>
@ibuclaw ibuclaw mentioned this pull request Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

8 participants