-4

I was writing a small Least common multiple algorithm and encountered something I don't understand. This is the first and last part of the code:

long a = 14159572;
long b = 63967072;
int rest = 4;
long long ans;
.
. // Some other code here that is not very interesting.
.
    else
{
    //This appears correct, prints out correct answer. 

    ans = b/rest;
    std::cout << a*ans;
}

But if I change the last "else" to this it gives an answer that is much smaller and incorrect:

    else
{

    std::cout << a*(b/rest);
}

Anyone know why this is? I don't think it's an overflow since it was no negative number that came out wrong, but rather just a much smaller integer (around 6*10^8) than the actual answer (around 2.2*10^14). As far as I understand it should calculate "b/rest" first in both cases, so the answers shouldn't differ?

Knut Knutsson
  • 13
  • 1
  • You missed to provide the actual code you wrote – Dutow Jul 28 '16 at 19:38
  • I believe the compiler reads left to right. So it may be distributing the "a" across the parens and giving a slightly different answer. – Winter Jul 28 '16 at 19:38
  • 3
    The type of `a*ans` is `long long` but the type of `a*(b/rest)` is just `long`. – user975989 Jul 28 '16 at 19:39
  • 4
    Provide a [MCVE] please, that actually enables everyone to reproduce your problem easily. Otherwise it's not very probable you get helped here. Thank you. – πάντα ῥεῖ Jul 28 '16 at 19:39
  • 1
    @Winter actually, in many cases, order of evaluation is undefined - for example: `foo(a(), b(), c());` - the compiler may call the 3 functions in *any* order before entering `foo`. – Jesper Juhl Jul 28 '16 at 19:41
  • `I don't think it's an overflow since it was no negative number that came out wrong` Overflow is undefined behavior for signed types. Have you checked your nostrils? – user975989 Jul 28 '16 at 19:43
  • When you come across a problem like this, try getting out of your chair and move around for 5 minutes, then come back to it. I think this a good question, though it is possible (I have no way of knowing, of course) that you did not look at your code for a very long time before posting it. The SO community does not like it when you don't spend a good 10 hours debugging the code. It should be understood though, that there are beginners on this site and it is silly to down vote their questions just because they are not professionals. –  Jul 28 '16 at 19:51
  • @code0 Are you actually trying to advise a strategy how to get rid of the _PEBCAC_ problem? :) – πάντα ῥεῖ Jul 28 '16 at 19:53
  • @user975989 They are both `long long`. Your second claim doesn't make sense and contradicts fpyour first. – user207421 Jul 28 '16 at 19:54
  • @EJP They aren't? http://coliru.stacked-crooked.com/a/52630e8ea59fcc68 – user975989 Jul 28 '16 at 20:00
  • 1
    @πάνταῥεῖ The problem can also be in the chair, as it is with bean bag chairs:) –  Jul 28 '16 at 20:02
  • Am a beginner at c++ and a beginner at this site, so to speak. Figured I provided all the numbers and code that were needed as a minimal and reproducible example of the specific problem, but maybe I should have restructured the code a bit and just put it inside a simple main(). Sorry about that, and thanks for taking the time:) – Knut Knutsson Jul 28 '16 at 20:57

3 Answers3

2

Difference is not order of operations but data types:

ans = b/rest; // b/rest is long which upscaled to long long
std::cout << a*ans; // a converted to long long and result is long long

vs:

std::cout << a*(b/rest); // a*(b/rest) all calculations in long

so if you change your second variant to:

std::cout << a*static_cast<long long>(b/rest);

you should see the same result.

Update to why your cast did not work, note the difference:

long a,b;
// divide `long` by `long` and upscale result to `long long`
std::cout << static_cast<long long>( a / b ); 
// upscale both arguments to `long long` and divide `long long` by `long long`
std::cout << a / static_cast<long long>( b );
Slava
  • 43,454
  • 1
  • 47
  • 90
  • Would it be better to just use `int64_t`? – Christopher Schneider Jul 28 '16 at 20:06
  • That question is for OP, not for me – Slava Jul 28 '16 at 20:08
  • Thank you! Looked for quite a while for an example of how data types were converted, I suspected the problem to be something along those lines. I did try to do the following before which did not work: `std::cout << long long (a*(b/rest))` But I guess that is not a correct way of making it a long long... – Knut Knutsson Jul 28 '16 at 21:01
  • @KnutKnutsson it is not incorrect way, it just has different effect see updated answer – Slava Jul 28 '16 at 21:23
2

You're still encountering overflow. Just because you're not observing a negative number doesn't mean there's no overflow.

In your case specifically, long is almost certainly a 32-bit integer, as opposed to long long which is probably a 64-bit integer.

Since the maximum value of a 32-bit signed integer is roughly 2 billion, 14159572 * (63967072 / 4) is most definitely overflowing the range.

Make sure you perform your calculations using long long numbers, or else reconsider your code to avoid overflow in the first place.

Xirema
  • 19,889
  • 4
  • 32
  • 68
0

The compiler assumes data types for each operand of your math equation and does the multiplication and division according to those assumed data types (refer to "integer division"). This also applies to intermediates of the computation. This also applies to the result passed to the stream since you don't pass a variable of an explicitly defined type.

Kyle Sweet
  • 306
  • 1
  • 3
  • 15