Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Make deprecation warnings compile errors #1982

Merged
thewilsonator merged 6 commits into from
Apr 8, 2019
Merged

Make deprecation warnings compile errors #1982

thewilsonator merged 6 commits into from
Apr 8, 2019

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Dec 2, 2017

This is already the way we build Phobos + Tools.
See also: dlang/phobos#5546

CC @ZombineDev @CyberShadow

@dlang-bot
Copy link
Contributor

dlang-bot commented Dec 2, 2017 •
edited
Loading

Thanks for your pull request, @wilzbach!

Bugzilla references

Auto-close Bugzilla Severity Description
✓ 18530 regression [Reg 2.079] src/rt/tracegc.d(43): Deprecation: The delete keyword has been deprecated
✗ 19789 normal Lazy expression evaluation doesn't check parent scope for deprecations

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 + druntime#1982"

@wilzbach wilzbach added the Trivial typos, formatting, comments label Dec 2, 2017
@CyberShadow
Copy link
Member

CyberShadow commented Dec 3, 2017

Sounds good, currently fails CI though.

@wilzbach
Copy link
Contributor Author

wilzbach commented Dec 12, 2017

Sounds good, currently fails CI though.

Yeah:

src/core/time.d(4581): Deprecation: struct core.time.FracSec is deprecated - FracSec has been deprecated in favor of just using Duration for the sake of simplicity
src/core/time.d(3594): Error: template instance core.time._assertThrown!(TimeException, FracSec) error instantiating

Easy to fix.

src/core/internal/arrayop.d(147): Error: static assert  "Unary `-` not supported for type `ubyte`."

That one turns out to be rather difficult as it appears deep in the arrayop mixin :/

@wilzbach wilzbach mentioned this pull request Dec 16, 2017
Merged
@dlang-bot dlang-bot added Needs Rebase needs a `git rebase` performed and removed Needs Rebase needs a `git rebase` performed labels Jan 1, 2018
@wilzbach wilzbach mentioned this pull request Feb 8, 2018
Merged
@wilzbach wilzbach mentioned this pull request Feb 23, 2018
Closed
@wilzbach wilzbach force-pushed the deprecation-error branch from c9eeea7 to e74740f Compare March 12, 2018 13:45
@wilzbach wilzbach requested a review from andralex as a code owner March 12, 2018 13:45
@wilzbach wilzbach force-pushed the deprecation-error branch from e74740f to dc7738d Compare March 12, 2018 13:47
@wilzbach
Copy link
Contributor Author

wilzbach commented Mar 12, 2018

Updated to resolve the merge conflicts and incorporate newer deprecations.
For delete dlang/dmd#8016 is required, apart from that I still haven't figured out how to fix the deprecation in arrayop.

@wilzbach wilzbach force-pushed the deprecation-error branch from dc7738d to ba520a3 Compare March 13, 2018 11:21
@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Mar 13, 2018
@wilzbach wilzbach force-pushed the deprecation-error branch from ba520a3 to 95c6043 Compare April 4, 2019 10:51
@wilzbach wilzbach mentioned this pull request Apr 4, 2019
Merged
@JinShil JinShil mentioned this pull request Apr 4, 2019
Closed
@wilzbach wilzbach force-pushed the deprecation-error branch from ac588cb to b53d839 Compare April 4, 2019 11:06
wilzbach
wilzbach commented Apr 4, 2019
View reviewed changes
@wilzbach wilzbach force-pushed the deprecation-error branch 2 times, most recently from 350c467 to 2c03d72 Compare April 4, 2019 11:42
@wilzbach wilzbach force-pushed the deprecation-error branch from 2c03d72 to b39d5d1 Compare April 4, 2019 11:44
wilzbach
wilzbach commented Apr 4, 2019
View reviewed changes
src/core/internal/arrayop.d
alias UT = Args[idx - 1];
enum op = Args[idx][1 .. $];
static if (is(typeof((UT a) => mixin(op ~ " a")) RT == return))
static if (is(typeof((UT a) => mixin(op ~ "cast(int) a")) RT == return))
Copy link
Contributor Author

@wilzbach wilzbach Apr 4, 2019

Choose a reason for hiding this comment

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

I could also have used -preview=intpromote here, but as this wouldn't be there when any user code tries to use this, I went with the "safe" approach of explicitly using the existing behavior.

@wilzbach
Copy link
Contributor Author

wilzbach commented Apr 5, 2019

@thewilsonator @JinShil FYI - this is passing and ready ;-)

@thewilsonator thewilsonator merged commit d7b99e9 into dlang:master Apr 8, 2019
@quickfur
Copy link
Member

quickfur commented Apr 8, 2019

Heads up: this PR has caused a regression in my code. Trying to reduce it to minimal case now, will post details once I reduced it.

@quickfur
Copy link
Member

quickfur commented Apr 9, 2019

Regression filed: https://issues.dlang.org/show_bug.cgi?id=19796

This PR broke array ops (arr[] = -src[];).

@wilzbach wilzbach deleted the deprecation-error branch April 9, 2019 04:36
quickfur
quickfur reviewed Apr 9, 2019
View reviewed changes
src/core/internal/arrayop.d
{
auto op = arg[0] == 'u' ? arg[1 .. $] : arg;
stack[$ - 1] = op ~ stack[$ - 1];
stack[$ - 1] = op ~ "cast(int)"~ stack[$ - 1];
Copy link
Member

@quickfur quickfur Apr 9, 2019

Choose a reason for hiding this comment

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

This is the culprit. Deleting "cast(int)"~ fixes the problem.

I think there's a missing check that Args contains integral types! The cast causes problems when performing array ops on double[].

@wilzbach
Copy link
Contributor Author

wilzbach commented Apr 9, 2019

Regression filed: https://issues.dlang.org/show_bug.cgi?id=19796

Thanks -> #2550

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@andralex andralex Awaiting requested review from andralex

@Burgos Burgos Awaiting requested review from Burgos

@rainers rainers Awaiting requested review from rainers

@schveiguy schveiguy Awaiting requested review from schveiguy

@CyberShadow CyberShadow Awaiting requested review from CyberShadow

@jmdavis jmdavis Awaiting requested review from jmdavis

@MartinNowak MartinNowak Awaiting requested review from MartinNowak

@WalterBright WalterBright Awaiting requested review from WalterBright

@PetarKirov PetarKirov Awaiting requested review from PetarKirov

5 more reviewers

@jacob-carlborg jacob-carlborg jacob-carlborg left review comments

@quickfur quickfur quickfur left review comments

@Geod24 Geod24 Geod24 left review comments

@JinShil JinShil JinShil left review comments

@thewilsonator thewilsonator thewilsonator approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug Fix Include reference to corresponding bugzilla issue Trivial typos, formatting, comments

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@wilzbach @dlang-bot @CyberShadow @quickfur @jacob-carlborg @Geod24 @JinShil @thewilsonator