Skip to content

Fix issue 9999,10560 - Deprecate the implicit int to bool conversion for integer literals#6404

Closed
LemonBoy wants to merge 2 commits intodlang:masterfrom
LemonBoy:b9999
Closed

Fix issue 9999,10560 - Deprecate the implicit int to bool conversion for integer literals#6404
LemonBoy wants to merge 2 commits intodlang:masterfrom
LemonBoy:b9999

Conversation

@LemonBoy
Copy link
Contributor

@LemonBoy LemonBoy commented Jan 6, 2017

As stated by @MartinNowak in #1942 the implicit conversion from int to bool should be removed and today, 4 years later.
A single PR to phobos [1] and one to DRuntime [2] are needed.

[1] dlang/phobos#5019
[2] dlang/druntime#1732

@dlang-bot
Copy link
Contributor

Fix Bugzilla Description
9999 Integer literal 0 and 1 should prefer integer type in overload resolution

@mathias-lang-sociomantic
Copy link
Contributor

s/Removed/Deprecated/

@LemonBoy
Copy link
Contributor Author

LemonBoy commented Jan 6, 2017

IMO the deprecation phase is long overdue heh
If you strongly feel there should be a deprecation phase please give me a timeline for the definitive removal so I can add it to the deprecation message

@mathias-lang-sociomantic
Copy link
Contributor

IMO the deprecation phase is long overdue heh

Sadly this approach doesn't scale. The reason we have this policy about deprecation is to keep a sane ecosystem package. More than often, you find a library that will only compile with a specific version of DMD, because someone felt a deprecation was not necessary for this specific case.

The "fix your code" approach does not scale either: often libraries have dependencies, which might not be compatible with the same frontend. You might also be blocked from updating because of a compiler regression. In general, we should never force working code to be changed if there is a simple deprecation path, and there is one here.

If you strongly feel there should be a deprecation phase please give me a timeline for the definitive removal so I can add it to the deprecation message

We don't currently have timeline for removal for any of the other deprecations. But it will be a couple of releases for sure.

@LemonBoy
Copy link
Contributor Author

LemonBoy commented Jan 6, 2017

Alright, you have a point, the only price to pay is having to show the deprecation warning twice but I don't think that's a dealbreaker here.

@LemonBoy LemonBoy changed the title Fix issue 9999,10560 - Remove the implicit int to bool conversion for integer literals Fix issue 9999,10560 - Deprecate the implicit int to bool conversion for integer literals Jan 6, 2017
@UplinkCoder
Copy link
Member

@LemonBoy do you have a compelling argument to deprecate this.
Which errors are avoided ?

@LemonBoy
Copy link
Contributor Author

LemonBoy commented Jan 6, 2017

The two Bugzilla issues referenced in the title contain enough examples of how this (mis)feature gives you a WTF moment.

@MetaLang
Copy link
Member

MetaLang commented Jan 6, 2017

@UplinkCoder

import std.stdio;
void foo(bool b) { writeln("bool"); }
void foo(long l) { writeln("long"); }
void main()
{
    foo(0); //bool?!
    foo(1); //bool?!
    foo(2); //long, okay
}

@UplinkCoder
Copy link
Member

We could fix that without deprecating the implicit cast.

@MartinNowak
Copy link
Member

Mmh, definitely yes for making that an ambiguous overload, but not too convinced of deprecating bool(0), bool(1), bool(i32 & 1)...

@LemonBoy
Copy link
Contributor Author

I see your point but I also think that conflating the integral types and bool is not such a wise idea for the very same reason there's a distinct bool type.
I don't have a strong opinion about this though.

@ntrel
Copy link
Contributor

ntrel commented May 4, 2017

Changing just the overload rules was the original idea from bugzilla, but the initial pull:
#1942

got closed to "drop implicit int literal to bool conversion instead".

@JinShil
Copy link
Contributor

JinShil commented Nov 19, 2017

Revived at #7310.

@JinShil JinShil closed this Nov 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

Comments