Issue 12228 & Issue 13943 - Deprecate usage of 'this' and 'super' as types #8242

JinShil · 2018-05-12T16:05:50Z

This is also a followup to #8141

dlang-bot · 2018-05-12T16:05:53Z

Thanks for your pull request, @JinShil!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✗	12228	normal	Identifiers 'this' and 'super' should not be allowed as base classes

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

JinShil · 2018-05-13T00:42:34Z

@CyberShadow, can you please advise on why DAutoTest is failing while others are passing?

CyberShadow · 2018-05-13T00:45:16Z

Could be due to a difference in the DMD version used to compile DMD (DAutoTest is trying to build the DMD source documentation at the point where it fails).

Does the error message make sense to you?

dmd/sapply.d(30): Error: typeof(super).visit is used as a type
dmd/nogc.d(32): Error: typeof(super).visit is used as a type
dmd/typesem.d(1513): Error: typeof(super).visit is used as a type
dmd/typesem.d(2059): Error: typeof(super).visit is used as a type
dmd/typesem.d(2562): Error: typeof(super).visit is used as a type
dmd/func.d(71): Error: typeof(super).visit is used as a type
dmd/apply.d(32): Error: typeof(super).visit is used as a type

If it's an easy fix, it might as well be done, to avoid unnecessarily restraining the range of versions that DMD can be built with.

JinShil · 2018-05-13T01:13:00Z

Does the error message make sense to you?

Somewhat. I did modify those lines, but it appears that the compiler being used to build the documentation has a bug.

DAutoTest: HOST_DC=/home/dtest/DAutoTest/work/dl/dmd-2.067.1/dmd2/linux/bin64/dmd
auto-tester: HOST_DC=/home/braddr/sandbox/at-client/release-build/dmd-2.079.0/linux/bin64/dmd

So DAutoTest is using 2.067.1? Is that necessary? Can it be upgraded?

wilzbach · 2018-05-13T01:14:22Z

src/dmd/typesem.d

        //printf("TypeIdentifier::resolve(sc = %p, idents = '%s')\n", sc, mt.toChars());
        if ((mt.ident.equals(Id._super) || mt.ident.equals(Id.This)) && !hasThis(sc))
        {
+            // @@@DEPRECATED_2019-05@@@.


The new deprecation DIP suggests to use versions instead.

CyberShadow · 2018-05-13T01:18:21Z

I'm looking into this.

BTW, any plans to deprecate/remove calling static methods via instances? Or at least opCall... I was bitten by this just yesterday, trying to wrap a class that had a non-static opCall.

JinShil · 2018-05-13T01:20:43Z

BTW, any plans to deprecate/remove calling static methods via instances?

Is there an bug/issue report?

CyberShadow · 2018-05-13T01:21:26Z

Sorry, I don't know. It's a very long standing pain point (since D 1.x).

JinShil · 2018-05-13T01:23:41Z

Sorry, I don't know. It's a very long standing pain point (since D 1.x).

File a bug report, get @WalterBright or @andralex to approve it, and if I can understand it and it's within my ability, I'll implement it.

CyberShadow · 2018-05-13T01:30:43Z

Found the issue.

DAutoTest and related software will use 2.079.0 once #8112 is merged (see #8112 (comment)). However, that PR is still not merged, so the old default (2.067.1) remains in force.

There are a few ways to proceed:

Wait until needed compiler features to convert backend to D #8112 is merged
Work around the compiler bug by adjusting the code in this PR
Add a second condition for detecting when a newer compiler is needed to ae.sys.d.manager to allow the code as it is in this PR right now to compile in Digger and DAutoTest.

I would obviously prefer one of the first two :)

wilzbach · 2018-05-13T01:31:46Z

deprecate/remove calling static methods via instances?

AFAICT that's a feature by design. I at least remember Walter defending this a few months ago as it allows for more generic code.

Or at least opCall

Not sure even this will be easy.
It's still used in Phobos: https://github.com/dlang/phobos/search?utf8=✓&q="static+opcall"&type=

CyberShadow · 2018-05-13T01:33:19Z

Being able to forbid non-static calls in a contract would work too.

JinShil · 2018-05-13T01:37:39Z

Will wait on #8112 or alternative.

WalterBright · 2018-05-21T05:34:56Z

AFAICT that's a feature by design. I at least remember Walter defending this a few months ago as it allows for more generic code.

Yes, it's deliberate. Removing it would require a compelling argument.

CyberShadow · 2018-05-21T07:39:20Z

Removing it would require a compelling argument.

The reason I asked is that I ran into a problem caused by it when trying to implement reference-counted classes.

The problem occurs when the wrapped class defines opCall. In this case, we cannot use static opCall in the reference-counted wrapper to wrap the class constructor (and we can't use normal struct constructors because this() without parameters is forbidden on structs).

In the end I had to move all construction to factory functions because of this issue.

Do you know a better solution?

JinShil · 2018-05-31T15:39:37Z

DAutoTest has been updated. This PR is ready to go.

Geod24 · 2018-06-12T15:45:15Z

There's a slight problem with this: https://issues.dlang.org/show_bug.cgi?id=18976

JinShil added the Review:WIP Work In Progress - not ready for review or pulling label May 12, 2018

jacob-carlborg approved these changes May 12, 2018

View reviewed changes

wilzbach approved these changes May 13, 2018

View reviewed changes

JinShil added Merge:Blocked and removed Review:WIP Work In Progress - not ready for review or pulling labels May 13, 2018

JinShil changed the title ~~Issue 12228 - Deprecate usage of 'this' and 'super' as types~~ Issue 12228 & 13943 - Deprecate usage of 'this' and 'super' as types May 14, 2018

JinShil changed the title ~~Issue 12228 & 13943 - Deprecate usage of 'this' and 'super' as types~~ Issue 12228 & Issue 13943 - Deprecate usage of 'this' and 'super' as types May 14, 2018

Issue 12228 - Deprecate usage of 'this' and 'super' as types

3905312

JinShil removed the Merge:Blocked label May 31, 2018

jacob-carlborg added the Merge:auto-merge label May 31, 2018

dlang-bot merged commit 524a924 into dlang:master May 31, 2018

JinShil mentioned this pull request Jun 2, 2018

Remove 'deprecate this as parameter type' changelog entry #8323

Merged

JinShil deleted the fix_12228 branch June 12, 2018 18:30

JinShil mentioned this pull request Aug 27, 2018

Fix Issue 18104: improve example showing that aliases cannot be used … dlang/dlang.org#2457

Merged

Uh oh!

Uh oh!

Issue 12228 & Issue 13943 - Deprecate usage of 'this' and 'super' as types #8242

Issue 12228 & Issue 13943 - Deprecate usage of 'this' and 'super' as types #8242

Conversation

JinShil commented May 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dlang-bot commented May 12, 2018

Bugzilla references

Testing this PR locally

Uh oh!

JinShil commented May 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CyberShadow commented May 13, 2018

Uh oh!

JinShil commented May 13, 2018

Uh oh!

wilzbach May 13, 2018

Choose a reason for hiding this comment

Uh oh!

CyberShadow commented May 13, 2018

Uh oh!

JinShil commented May 13, 2018

Uh oh!

CyberShadow commented May 13, 2018

Uh oh!

JinShil commented May 13, 2018

Uh oh!

CyberShadow commented May 13, 2018

Uh oh!

wilzbach commented May 13, 2018

Uh oh!

CyberShadow commented May 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JinShil commented May 13, 2018

Uh oh!

WalterBright commented May 21, 2018

Uh oh!

CyberShadow commented May 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JinShil commented May 31, 2018

Uh oh!

Geod24 commented Jun 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

JinShil commented May 12, 2018 •

edited

Loading

JinShil commented May 13, 2018 •

edited

Loading

CyberShadow commented May 13, 2018 •

edited

Loading

CyberShadow commented May 21, 2018 •

edited

Loading