fix Issue 16997 - Integral promotion rules not being followed for neg…

[WalterBright]

…ation expressions

Not following integral promotion rules is a serious bug. Don't pull this yet, I want to see what impact fixing this has first.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Description
✓	16997	Integral promotion rules not being followed for unary + - ~ expressions

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

[WalterBright]

Blocked by dlang/phobos#5646

[dnadlinger]

Perhaps -transition=intpromote for a little bit more of a hint as to the meaning when users encounter the flag in makefiles/…?

[WalterBright]

ok

[MartinNowak]

-transition=intpromote

[MartinNowak]

This breaks lots of packages because this PR is using warnings instead of deprecations. This is an outdated practice from before the -dw and -de switches were added, and deprecations were changed to non-fatal by default.
Most people (and currently dub by default) treat warnings as fatal and deprecations as non-fatal, following dmd's no-noise warnings approach.
Also, fairly simple, we should handle deprecations in the language like those in libraries.

Also see dlang/dub#493

[WalterBright]

The new arrayop stuff you wrote breaks with this:

src/core/internal/arrayop.d-mixin-56(56): Warning: integral promotion not done for `-_param_2[pos]`, use '-transition=promote' switch or `-cast(int)(_param_2[pos])`

It's a bit hard to see where that is coming from in arrayop.d - can you have a look, please? In general, the idea is to replace -x with 0-x and ~x with -1-x

[MartinNowak]

It's a bit hard to see where that is coming from in arrayop.d - can you have a look, please? In general, the idea is to replace -x with 0-x and ~x with -1-x

Well the existing arrayop as well as the new implementation only perform a single cast.

// byte[] = -byte[];
foreach (i; 0 .. result.length)
  result[i] = cast(byte)(-a[i]);

// byte[] = -byte[] + byte[];
foreach (i; 0 .. result.length)
  result[i] = cast(byte)(-a[i] + b[i]);

There is one final cast before assigning that deals with intermediate integer promotions.
So it's somewhat of a false error of your deprecation. Could you check whether the expressions gets promoted or cast anyhow?
It's somewhat laborious to add a workaround for this, as arrayops support arbitrary mixed types. I'd need to change the architecture of the codegen to run type inference of sub-expressions alongside the codegeneration in order to be able to add special code for some type+operation combinations.

[andralex]

So what's the outlook on this? Deprecate or warn? I'm okay with each seeing that each approach has been massively used by various languages. Since we're not making this language change as part of a major general language update, deprecation seems a lot more sensible. @WalterBright can you please use deprecation?

[WalterBright]

Blocked by dlang/phobos#5793

[WalterBright]

@andralex it's using deprecation now, and is passing.

[schveiguy]

Note, the changelog here didn't reflect the update to doing deprecations instead of warnings. See https://issues.dlang.org/show_bug.cgi?id=18148

[Geod24]

This deprecation triggers on valid code and is fairly annoying. See https://issues.dlang.org/show_bug.cgi?id=20678