Converting integers with different capacity

I have the following code:

// ---- third party library code ----
struct Point { int16_t x, y; };
void third_party_library__getPoint(Point*);
void third_party_library__setPoint(const Point*);

// ---- my library code ----
void move_point(int dx, int dy)
{
    Point pt;
    third_party_library__getPoint(&pt);
    pt.x += dx;
    pt.y += dy;
    third_party_library__setPoint(&pt);
}

Line pt.x += dx; yields warning

conversion from 'int' to 'int16_t', possible loss of data

What should I do?

1. disa开发者_如何学Goble warning for those lines
2. move the source of warning to interface: make dx and dy int16_t, so one who will use move_point function will deal with this problem.
3. just cast dx and dy to int16_t.
4. add assert(dx <= 0x7FFF && dx >= -0x8000 && "too large 'dx' value") and hope that it will hit when I will run debug version.

You should make dx and dy int16_t, if you cast them you're just hiding it and the person using your code won't see those warnings and be unaware of the problem. So let the person on the other side handle the warnings, at least they are sure of the functionality then.

For me there are 3 related but different points here:

A. Deal with the overflow

In fact, there are two parts here: conversion overflow and addition overflow. Even if you take int16_t as the input, there could still be an overflow at the += stage, and you have to decide what to do with it (of course you can simply ignore it, but this is a choice too, with well-defined consequences).

B. Inform users about limitations

By choosing your (3) option - moving to the interface, you would inform the users about the first overflow but not about the second one. Also, it is not the only way to inform users. Sometimes a better way is to have "limitations" section in your documentation, to make it clear right away.

C. Avoid the warning

Once you have decided with (A), you can make the compiler happy by making the conversion according to what you decide.

Your (1) and (3) is essentially the same thing, though people don't like warning suppression much. These options only answer Avoid, but not Deal nor Inform.

(2) ignores the second overflow, and sends the first one to the user. So it does Avoid and partially Inform, ignoring a half of Deal and leaving the second half to the users.

(4) is only about Inform, but has nothing to do with Deal and Avoid.

What would I do? First I would decide on how to Deal with the overflow - depending on the application it can be quite different, i.e. it could be the standard overflow behavior or truncation to the maximum possible value. Then I would think on how to properly Inform my users - this depends on the way the docs are organized, if asserts/exceptions are acceptable etc. Finally, I would Avoid the warning according to that, by checking for overflow and acting according to the Deal strategy.

Assuming you know for sure that there isn't actually a bug here (the variable will never have bits past bit 15 that you care about), I'd go with some variant of "casting".

What I usually do in this case for two integer types is a constructor:

pt.x += int16_t(dx);

That should make it pretty clear to anyone reading this code that yes indeed you do in fact want to discard any bits past the 16th.

If your implementation can't cope with over int16_t, then you should take int16_t.

I prefer 4, however the best choice depends on context (e.g. performance considerations).

1. disable warning for those lines

   This is bad because disabling warnings adds dirt to your code (e.g. the syntax for disabling warnings depends on compiler)

2. move the source of warning to interface: make dx and dy int16_t, so one who will use move_point function will deal with this problem

   This is bad because you might want to upgrade your third-party library in future without changing your library's interface

3. just cast dx and dy to int16_t.

   This is bad because it might silently cause an overflow (obviously).

   BTW just using pt.x += int16_t(dx) will not "fix" the warning; if you want to hide the warning, use pt.x = int16_t(pt.x + dx) (or some alternative casting syntax)

4. add assert(dx <= 0x7FFF && dx >= -0x8000 && "too large 'dx' value") and hope that it will hit when I will run debug version.

   This is some hairy syntax, but i guess this is the least evil (its only drawback, except syntax, is performance, and we can probably ignore it). You can also use the following syntax (avoiding magic numbers):

   assert(int16_t(dx) == dx);
   

add assert(dx <= 0x7FFF && dx >= -0x8000 && "too large 'dx' value") and hope that it will hit when I will run debug version.

Well, obviously you must handle the "potential" error (I'd call it a full blown error, btw).

In order to accept the warning (knowing only 16-bit numbers will be passed to your function) you can add a cast, which expresses what you are doing when throwing away the 16 higher bits anyway.

All this said assuming you have a 32 or 64 bit platform.