Bug 16997 – Integral promotion rules not being followed for unary + - ~ expressions

Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-12-21T00:18:49Z
Last change time
2018-02-05T22:41:15Z
Assigned to
No Owner
Creator
ponce
See also
https://issues.dlang.org/show_bug.cgi?id=17637, https://issues.dlang.org/show_bug.cgi?id=5132, https://issues.dlang.org/show_bug.cgi?id=18148, https://issues.dlang.org/show_bug.cgi?id=18380

Attachments

IDFilenameSummaryContent-TypeSize
16260001-wunary-to-warn-on-unary-minus-plus-on-small-types.patch.txtPatch in ketmar DMD fork ("aliced") to make unary +/- on small integers a warningtext/plain3234
16270001-warn-on-unary-minus-plus-vanilla.patchwarning on unary minus/plustext/plain3340

Comments

Comment #0 by aliloko — 2016-12-21T00:18:49Z
Consider the following C++ program: ----- #include <cstdio> int main() { signed char c = -128; printf("%d\n", (int)(-c)); return 0; } ----- This prints "128". Consider the supposedly identical D code: ----- import core.stdc.stdio; void main() { byte c = -128; printf("%d\n", cast(int)(-c)); // -128 } ----- Since the D unary minus doesn't promote byte to int, it yields a different result. This is a real case that costed ketmar 40 min, while porting working C code. Porting to D silently broke the code due to subtly different integer promotion rules. I'm pretty sure I would have given up becasue code that is translated is often enough not understood yet. After investigation: * Taking the unary minus of byte and short and then asting to int yield different results for the lowest values: -128 and -32768 * Taking the unary minus of ubyte and ushort and then casting to int yield different results for any value except 0 Why are the promotion rules for unary + and - different from C?
Comment #1 by aliloko — 2016-12-21T00:32:04Z
Created attachment 1626 Patch in ketmar DMD fork ("aliced") to make unary +/- on small integers a warning
Comment #2 by ketmar — 2016-12-21T04:58:18Z
Created attachment 1627 warning on unary minus/plus fixed patch, made against vanilla. it will show warning for unary -/+ for (u)byte and (u)short, but one can silence that with `-(expr)`. it prolly needs better message with explanation of what it is, and what to do, and why -- but i leave that for that one who will make a PR out of the patch.
Comment #3 by ketmar — 2016-12-21T05:02:24Z
as for the question: promotion rules are what they are currently to allow this: byte n; n = -n; if unary minus will promote `n`, you will need to add `cast` there. and negate never ever drop bits from integer (even byte.min is essentially unchanged, so no info is lost), so it is -- relatively -- safe to not promote here.
Comment #4 by ketmar — 2016-12-21T05:03:34Z
p.s.: i'm not that smart, tbh, i understood the rationale behind the design after alot of hard thinking.
Comment #5 by bugzilla — 2017-07-22T09:48:58Z
Comment #6 by bugzilla — 2017-07-22T09:50:33Z
*** Issue 17637 has been marked as a duplicate of this issue. ***
Comment #7 by bugzilla — 2017-07-22T10:27:41Z
Unary + and unary ~ are also broken in the same way.
Comment #8 by bugzilla — 2017-07-22T10:35:53Z
Note that the spec says: "Note: unlike in C and C++, the usual integral promotions are not performed prior to the complement operation." http://dlang.org/spec/expression.html#complement_expressions And the spec says nothing about unary - or unary +. http://dlang.org/spec/expression.html#unary-expression
Comment #9 by github-bugzilla — 2017-07-24T23:08:43Z
Commits pushed to master at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/47d9a09e66d7fde929b596a72450a92d51d0b3e1 bigint: fix dependency on broken Issue 16997 https://github.com/dlang/phobos/commit/c1832981e3818a151ee25805d47d472633a218ad Merge pull request #5646 from WalterBright/bigint-cast bigint: fix dependency on broken Issue 16997 merged-on-behalf-of: Andrei Alexandrescu <[email protected]>
Comment #10 by github-bugzilla — 2017-08-16T13:24:15Z
Commits pushed to stable at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/47d9a09e66d7fde929b596a72450a92d51d0b3e1 bigint: fix dependency on broken Issue 16997 https://github.com/dlang/phobos/commit/c1832981e3818a151ee25805d47d472633a218ad Merge pull request #5646 from WalterBright/bigint-cast
Comment #11 by github-bugzilla — 2017-10-23T21:59:18Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/71403b6cd3781cd4d97104be1f4194e168a75d02 fix Issue 16997 - Integral promotion rules not being followed for unary + - ~ expressions https://github.com/dlang/dmd/commit/7253190083cd66ae11711f2f6ff15497810d34c5 Merge pull request #7013 from WalterBright/fix16997 fix Issue 16997 - Integral promotion rules not being followed for neg…
Comment #12 by github-bugzilla — 2017-12-18T22:55:09Z
Commits pushed to stable at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/71403b6cd3781cd4d97104be1f4194e168a75d02 fix Issue 16997 - Integral promotion rules not being followed for unary + - ~ expressions https://github.com/dlang/dmd/commit/7253190083cd66ae11711f2f6ff15497810d34c5 Merge pull request #7013 from WalterBright/fix16997
Comment #13 by github-bugzilla — 2018-01-05T13:30:35Z
Commits pushed to dmd-cxx at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/47d9a09e66d7fde929b596a72450a92d51d0b3e1 bigint: fix dependency on broken Issue 16997 https://github.com/dlang/phobos/commit/c1832981e3818a151ee25805d47d472633a218ad Merge pull request #5646 from WalterBright/bigint-cast
Comment #14 by aliloko — 2018-01-05T13:43:47Z
Great! D is now a bit easier to explain.