Turn informational messages of transition=complex into deprecated #7081

ibuclaw · 2017-08-13T19:28:03Z

I think it's high time to push forward with this.

Also ironed out a small problem I found when building phobos with this turned on.

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

dlang-bot · 2017-08-13T19:28:04Z

Thanks for your pull request, @ibuclaw!

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.

wilzbach · 2017-08-13T21:06:40Z

I don't think people will use this flag, so what stops us from turning it into a deprecation message for everyone?

ibuclaw · 2017-08-13T21:12:22Z

I don't think people will use this flag, so what stops us from turning it into a deprecation message for everyone?

And identifying and removing (or fixing) all uses of complex or imaginary types in druntime and phobos.

WalterBright · 2017-08-14T04:17:53Z

src/ddmd/mtype.d

+            t = (cast(TypeEnum)t).sym.memtype;
+
+        // Type is forward referenced or not a scalar type.
+        if (t is null || !t.isTypeBasic())


Is the check for typebasic really necessary? This should be covered by line 3076. Also, the null check seems unnecessary. Is there a test case for this? According to the previous logic, it could only be null if memtype is null. Can this ever happen?

WalterBright · 2017-08-14T04:20:16Z

src/ddmd/mtype.d

@@ -3091,11 +3098,13 @@ extern (C++) abstract class Type : RootObject
            }
            if (t.iscomplex())
            {
-                fprintf(global.stdmsg, "%s: use of complex type '%s' is scheduled for deprecation, use 'std.complex.Complex!(%s)' instead\n", p ? p : "", toChars(), rt.toChars());
+                deprecation(loc, "use of complex type `%s` is scheduled for deprecation, use `std.complex.Complex!(%s)` instead\n",


It's clearly not scheduled for deprecated, it is deprecated, as it calls deprecation()

WalterBright · 2017-08-14T04:22:00Z

test/compilable/sw_transition_complex.d

@@ -134,3 +134,10 @@ creal test14488b()
    return 1 + 0i;
 }

+// Forward referenced types shouldn't cause spurious errors during test for complex or imaginary.
+enum E;


Is this the test case for the null t?

Yes, the error is in getMemType if I recall correctly.

WalterBright · 2017-08-14T04:22:40Z

test/compilable/sw_transition_complex.d

@@ -134,3 +134,10 @@ creal test14488b()
    return 1 + 0i;
 }

+// Forward referenced types shouldn't cause spurious errors during test for complex or imaginary.
+enum E;
+struct S;


I don't think this case is strictly necessary, but no problem leaving it in.

ibuclaw · 2017-08-28T06:11:27Z

Should rebase.

ibuclaw · 2017-09-28T21:10:34Z

Rebased. There's an old open PR in #5731 that I should look at for comparison to this.

mathias-lang-sociomantic · 2017-10-05T10:54:43Z

Why not make it a deprecation instead (without the switch) ?
This switch has been there for a while, people interested in it have plenty of DMD releases to use to fix it.

ibuclaw · 2017-10-06T07:25:47Z

@mathias-lang-sociomantic I found a small bug when building phobos with it turned on. So I'm not sure if people were using it.

I think it would be better to first push deprecations in phobos, and the compiler/druntime interface updated. Then turn on deprecation by default.

wilzbach

LGTM. What's blocking this?
FWIW I agree that the deprecation messages should be turned on by default as no one will be using this switch, but this can always be done in a separate PR ...

JinShil · 2017-12-27T05:21:08Z

@ibuclaw I'm itching to pull this. Is it ready to go? Is there anything outstanding?

JinShil · 2017-12-27T05:25:17Z

~~We also need an update to the deprecations table, but I don't want it to hold up this PR.~~

Probably should wait until turning deprecations on by default.

WalterBright · 2017-12-27T08:04:12Z

src/ddmd/mtype.d

+            t = (cast(TypeEnum)t).sym.memtype;
+
+        // Type is forward referenced.
+        if (t is null)


Enums are setup to be lazily analyzed, meaning if it is fwd referenced just run semantic() on it.

The comment should probably be anonymous enum. The ICE here was because an enum was declared but never defined. See test.

JinShil · 2018-01-04T21:03:43Z

@ibuclaw If this is ready to go, please rebase and I'll merge.

ibuclaw · 2018-01-05T08:08:40Z

There's no merge conflicts, why rebase?

I'll update the comment to clarify why there is a null check for enums.

@WalterBright are you blocking this?

ibuclaw · 2018-01-05T08:10:42Z

Ah, now the page is telling me that there are conflicts, and dlang bot has added a tag.

JinShil · 2018-01-05T08:11:42Z

There's no merge conflicts, why rebase?

The GitHub web interface seems to think there is a conflict for src/ddmd/mars.d and src/ddmd/mtype.d I think because of the transition from ddmd to dmd.

Either that or this is one of those old PRs that has reached GitHubs 1000 status limit and is no longer updated. In that case, a force push should reset it.

JinShil · 2018-01-06T10:06:34Z

@ibuclaw Sorry, but can you please rebase again? This is why we need to act swiftly on PRs that are ready to go.

If you want to wait for @WalterBright to sign off on this first, please explicitly say so or mark it as "Needs Approval", otherwise, I'll merge right after you rebase.

ibuclaw · 2018-01-06T10:32:38Z

@JinShil - rebased. If you think that all concerns have been addressed sufficiently, then go ahead and merge.

WalterBright reviewed Aug 14, 2017

View reviewed changes

ibuclaw force-pushed the depcomplex branch 2 times, most recently from 8de6634 to d308be3 Compare August 14, 2017 22:47

ibuclaw mentioned this pull request Aug 19, 2017

Add std.complex.coshisinh and remove uses of creal math. dlang/phobos#5698

Closed

ibuclaw force-pushed the depcomplex branch from d308be3 to 16df0e4 Compare August 19, 2017 14:20

ibuclaw force-pushed the depcomplex branch 2 times, most recently from e1f5808 to 1dd4787 Compare September 28, 2017 21:29

wilzbach approved these changes Dec 13, 2017

View reviewed changes

JinShil approved these changes Dec 27, 2017

View reviewed changes

WalterBright reviewed Dec 27, 2017

View reviewed changes

dlang-bot added the Needs Rebase label Jan 1, 2018

dlang-bot removed the Needs Rebase label Jan 6, 2018

ibuclaw force-pushed the depcomplex branch from 1dd4787 to 4aea7fd Compare January 6, 2018 10:26

ibuclaw force-pushed the depcomplex branch from 4aea7fd to 75695e6 Compare January 6, 2018 10:30

JinShil added the auto-merge label Jan 6, 2018

ibuclaw force-pushed the depcomplex branch from 75695e6 to decd961 Compare January 6, 2018 17:43

dlang-bot removed the auto-merge label Jan 6, 2018

Turn informational messages of transition=complex into deprecated

e8c06f3

ibuclaw force-pushed the depcomplex branch from decd961 to e8c06f3 Compare January 6, 2018 17:44

JinShil added the auto-merge label Jan 6, 2018

dlang-bot merged commit 67e6c2a into dlang:master Jan 6, 2018

ibuclaw deleted the depcomplex branch January 6, 2018 23:12

wilzbach mentioned this pull request Jan 8, 2018

Deprecate imaginary and complex types #7640

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turn informational messages of transition=complex into deprecated #7081

Turn informational messages of transition=complex into deprecated #7081

ibuclaw commented Aug 13, 2017 •

edited

dlang-bot commented Aug 13, 2017

wilzbach commented Aug 13, 2017

ibuclaw commented Aug 13, 2017

WalterBright Aug 14, 2017

WalterBright Aug 14, 2017

WalterBright Aug 14, 2017

ibuclaw Aug 14, 2017

WalterBright Aug 14, 2017

ibuclaw commented Aug 28, 2017

ibuclaw commented Sep 28, 2017

mathias-lang-sociomantic commented Oct 5, 2017

ibuclaw commented Oct 6, 2017

wilzbach left a comment

JinShil commented Dec 27, 2017

JinShil commented Dec 27, 2017 •

edited

WalterBright Dec 27, 2017

ibuclaw Dec 27, 2017

JinShil commented Jan 4, 2018

ibuclaw commented Jan 5, 2018

ibuclaw commented Jan 5, 2018

JinShil commented Jan 5, 2018

JinShil commented Jan 6, 2018 •

edited

ibuclaw commented Jan 6, 2018

Turn informational messages of transition=complex into deprecated #7081

Turn informational messages of transition=complex into deprecated #7081

Conversation

ibuclaw commented Aug 13, 2017 • edited

dlang-bot commented Aug 13, 2017

Bugzilla references

wilzbach commented Aug 13, 2017

ibuclaw commented Aug 13, 2017

WalterBright Aug 14, 2017

Choose a reason for hiding this comment

WalterBright Aug 14, 2017

Choose a reason for hiding this comment

WalterBright Aug 14, 2017

Choose a reason for hiding this comment

ibuclaw Aug 14, 2017

Choose a reason for hiding this comment

WalterBright Aug 14, 2017

Choose a reason for hiding this comment

ibuclaw commented Aug 28, 2017

ibuclaw commented Sep 28, 2017

mathias-lang-sociomantic commented Oct 5, 2017

ibuclaw commented Oct 6, 2017

wilzbach left a comment

Choose a reason for hiding this comment

JinShil commented Dec 27, 2017

JinShil commented Dec 27, 2017 • edited

WalterBright Dec 27, 2017

Choose a reason for hiding this comment

ibuclaw Dec 27, 2017

Choose a reason for hiding this comment

JinShil commented Jan 4, 2018

ibuclaw commented Jan 5, 2018

ibuclaw commented Jan 5, 2018

JinShil commented Jan 5, 2018

JinShil commented Jan 6, 2018 • edited

ibuclaw commented Jan 6, 2018

ibuclaw commented Aug 13, 2017 •

edited

JinShil commented Dec 27, 2017 •

edited

JinShil commented Jan 6, 2018 •

edited