Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 3841: silent implicit cast from floating point to integral in += etc. operators #4643

Merged
merged 2 commits into from May 19, 2015

Conversation

legrosbuffle
Copy link
Contributor

Note that this might break some existing code that relies on bad behaviour, but I think it's important to fix. For example, there are three such instances in phobos, and one of them is obviously bogus:

int chunksize, extra, numchunks;
...
chunksize += extra / cast(double)numchunks;

(I'll send a pull request to phobos)

@legrosbuffle legrosbuffle force-pushed the fix3841 branch 2 times, most recently from e58ccfe to b29f8f0 Compare May 11, 2015 16:59
@WalterBright
Copy link
Member

@WalterBright
Copy link
Member

The large function being moved from one place to another in the file makes it very, very difficult to review.

@legrosbuffle
Copy link
Contributor Author

I've un-moved the function

{
if ((type->isintegral() && t2->isfloating()))
{
error("%s %s %s is undefined (refusing to perform truncating conversion)",
Copy link
Member

Choose a reason for hiding this comment

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

This should be made a warning, because it will force people to change their code. The idea is warning, then deprecation, then finally error, after some major time has elapsed.

@WalterBright
Copy link
Member

Thanks for undoing the code movement. Now I can see what changed!

// pragma(msg, LHS, " += ", RHS);
LHS a;
RHS b;
a += b;
Copy link
Member

Choose a reason for hiding this comment

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

This is good, but also need to test -=, *=, /=, %= and ^^=

@andralex
Copy link
Member

thx!! please add the extra tests

@legrosbuffle
Copy link
Contributor Author

Done, thanks for the review.

@WalterBright
Copy link
Member

Auto-merge toggled on

WalterBright added a commit that referenced this pull request May 19, 2015
Issue 3841: silent implicit cast from floating point to integral in += etc. operators
@WalterBright WalterBright merged commit bd25986 into dlang:master May 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants