Skip to content

[New DIP] Deprecation and removal of implicit integer and character literal conversion to bool#99

Merged
mdparker merged 3 commits intodlang:masterfrom
JinShil:bool_conv
Jun 20, 2018
Merged

[New DIP] Deprecation and removal of implicit integer and character literal conversion to bool#99
mdparker merged 3 commits intodlang:masterfrom
JinShil:bool_conv

Conversation

@JinShil
Copy link
Contributor

@JinShil JinShil commented Nov 12, 2017

Still not quite ready, but should be in a few days.

@PetarKirov
Copy link
Member

PetarKirov commented Nov 13, 2017

We also need to consider array literals. There's code out there that explicitly relies on the implicit int->bool conversion. For example, BitArray's examples would look considerably less readable if we disallow that.

@JinShil
Copy link
Contributor Author

JinShil commented Nov 13, 2017

We also need to consider array literals. There's code out there that explicitly relies on the implicit int->bool conversion. For example, BitArray's examples would look considerably less readable if we disallow that.

Good point! I think we can overload the BitArray constructors to mitigate that specific case, but consider other cases like this:

bool[] values = [1,0,1,0,1,1];

Users can always do:

bool[] values = cast(bool[])[1,0,1,0,1,1];

That's not too bad, IMO, but I wonder how much disruption that would cause in the D ecosystem.

Anyway, I probably need to add this to the DIP. Thanks.

@PetarKirov
Copy link
Member

I wonder if it's a good idea to allow implicit conversion for functions that are not overloaded.

@JinShil
Copy link
Contributor Author

JinShil commented Nov 13, 2017

I wonder if it's a good idea to allow implicit conversion for functions that are not overloaded.

I assume you mean just for int-->bool, not in general, yes?

@PetarKirov
Copy link
Member

PetarKirov commented Nov 13, 2017

I assume you mean just for int-->bool, not in general, yes?

Yes, only when a function is not overloaded (and not a template) and the parameter is of type bool or a type that has alias boolField this.

@mdparker
Copy link
Member

Normally, I'd prefer for a DIP to be in a state that's ready to begin the Draft Review before the PR is submitted. No big deal, though, since that isn't documented anywhere at the moment. I'll just need you to let me know when this one and #97 are ready for Draft Review.

I also need to ask for your patience. There will likely be a bit of a delay before I initiate any new DIP reviews. We're currently looking at ways to improve upon the existing process. I don't want to start anything new until that's settled. Hopefully, it won't be too long before we get there.

Thanks!

@JinShil
Copy link
Contributor Author

JinShil commented Nov 14, 2017

@mdparker Understood. #97 is ready to go whenever you are.

@JinShil
Copy link
Contributor Author

JinShil commented Nov 14, 2017

Closing for now, as this needs quite a bit more work.

@JinShil JinShil closed this Nov 14, 2017
@JinShil
Copy link
Contributor Author

JinShil commented Nov 16, 2017

This is ready for Draft Review.

@JinShil JinShil reopened this Nov 16, 2017
@mdparker
Copy link
Member

mdparker commented Mar 7, 2018

@JinShil We can move on this one first if you prefer. Let me know. Whichever you pick to move on second will be delayed a bit longer. I'll want to see if any of the others are ready to go after moving your first one through.


## Abstract

This DIP proposes deprecation and removal of implicit conversion from integer and character literals to `bool` in order to close a hole in the type system and resolve overload resolution bugs. For the purpose of this document, *Integer literals* are literals of type `int`, `uint`, `long` or `ulong` and *Character literals* are literals of type `char`, `wchar`, or `dchar`.
Copy link
Member

Choose a reason for hiding this comment

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

"proposes deprecation" => proposes thedeprecation

"to bool in order to close" => "to bool, thereby closing

"and resolve" => and resolving


This DIP proposes deprecation and removal of implicit conversion from integer and character literals to `bool` in order to close a hole in the type system and resolve overload resolution bugs. For the purpose of this document, *Integer literals* are literals of type `int`, `uint`, `long` or `ulong` and *Character literals* are literals of type `char`, `wchar`, or `dchar`.

### Links
Copy link
Member

Choose a reason for hiding this comment

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

Please update the DIP to use the new template layout.

boolValue = 1UL;
}
```
The implicit conversion of character literals to `bool` is questionable, as there is no intrinsic relationship between characters `'\0'` and `'\1'`, and boolean states false and true. This behavior may actually be simply an unintended consequence of [integer promotion in D](https://dlang.org/spec/type.html#integer-promotions) and implicit integer literal conversion to `bool`. The reader of such code would be right to question whether such expressions are actually programming errors or the author's intent, as they circumvent the type system without an explicit cast.
Copy link
Member

Choose a reason for hiding this comment

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

"between characters" => between the characters

"and boolean" => and the boolean

"false and true" => false and true

"may actually be simply an" => may be an

```
The implicit conversion of character literals to `bool` is questionable, as there is no intrinsic relationship between characters `'\0'` and `'\1'`, and boolean states false and true. This behavior may actually be simply an unintended consequence of [integer promotion in D](https://dlang.org/spec/type.html#integer-promotions) and implicit integer literal conversion to `bool`. The reader of such code would be right to question whether such expressions are actually programming errors or the author's intent, as they circumvent the type system without an explicit cast.

It is not clear how the implicit conversion of integer literals to `bool` came to be a feature in D. It may be a historical leftover from a time when [D had a 1-bit integer, `bit`, that was later replaced by `bool`](https://issues.dlang.org/show_bug.cgi?id=9999#c9). It may also be a carry-over from D's roots in C, the way conditionals are evaluated in C, and the conventions that were employed over the years to use integer values to compensate for C's lack of a boolean type. Regardless, unlike C, D has a boolean type `bool`, boolean literals `false` and `true`, and also supports explicit casting of integers to `bool`, so implicit conversion from integer literals to `bool` is unnecessary, and even problematic as demonstrated below.
Copy link
Member

Choose a reason for hiding this comment

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

I think everything before and including "Regardless" can be deleted. Speculation on possible origins is superfluous. If you had something concrete against which you needed to justify the change, then that would be different. Maybe something will come up in review.

D's existing implementation implicitly converts integer and character literals that evaluate to 0 and 1 to `false` and `true` respectively.

### Example A
The following Example currently compiles in the existing implementation.
Copy link
Member

Choose a reason for hiding this comment

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

"Example" => example


Also, like the former, it would still break code relying on the existing overload resolution behavior illustrated in [Example B](#example-b) and [Example C](#example-c), resulting in a deprecation process similar, if not the same, to that of the proposed solution, but would not close the hole in the type system.

This solution was also not analyzed in enough detail to determine whether it would even scale.
Copy link
Member

Choose a reason for hiding this comment

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

"would even scale" => would scale


The proposed implementation is to be rolled out through a sequence of steps to minimize any disruption.

1. If and when this DIP is approved, an announcement will be made on the D programming language forum to raise awareness of the change in the community. Users will have a link to this DIP and the implementation so they can understand how to mitigate breakage in their code, and monitor the timing of the subsequent releases.
Copy link
Member

Choose a reason for hiding this comment

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

"If and when this DIP is approved, an"=> An


## Copyright & License

Copyright (c) 2017 by the D Language Foundation
Copy link
Member

Choose a reason for hiding this comment

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

"2017" => 2018


2. After approval of this DIP, code in DMD's test suite, Phobos, and DRuntime will be updated with the appropriate remedies documented in [Description](#description) to avoid deprecation warnings or errors in the stage 1 and stage 2 releases. Preemptive pull requests anticipating the change proposed in this DIP have already been merged for [Phobos](https://github.com/dlang/phobos/pull/5019) and [DRuntime](https://github.com/dlang/druntime/pull/1732), but another pass will need to be made.

4. After the changes in step 2 have been pulled, stage 1 can be pulled. From that point on the compiler will emit deprecation warnings when an implicit conversion from an integer literal to `bool` is encountered. This will precisely identify instances in users' code that will no longer compile when stage 2 is released, but will still perform the conversion, buying users time to update their code. The deprecation will remain in place for a time period to be decided by the language authors at the time of this DIP's formal review.
Copy link
Member

Choose a reason for hiding this comment

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

"been pulled, stage 1 can be pulled" => been merged, state 1 can be merged

"the language authors" => the language maintainers
That's the "official" terminology now.

"formal review" => formal assessment
Ditto.


This solution was also not analyzed in enough detail to determine whether it would even scale.

### Breaking changes / deprecation process
Copy link
Member

Choose a reason for hiding this comment

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

Expected breakage needs to be iterated here, even when it is mentioned elsewhere in the document.

@mdparker mdparker merged commit 19ca5a6 into dlang:master Jun 20, 2018
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.

3 participants