BIND 10 #3015: Change type of IntElement to int64_t

[BIND 10 Development]

#3015: Change type of IntElement to int64_t
-------------------------------------+-------------------------------------
            Reporter:  fujiwara      |                        Owner:
                Type:  defect        |  fujiwara
            Priority:  medium        |                       Status:
           Component:  Inter-module  |  reviewing
  communication                      |                    Milestone:
            Keywords:                |  Sprint-20130806
           Sensitive:  0             |                   Resolution:
         Sub-Project:  Core          |                 CVSS Scoring:
Estimated Difficulty:  1             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by fujiwara):

 Thanks for reviewing,

 Replying to [comment:10 vorner]:
 > Why do you use `int64_t` in some method parameters and `long long int`
 at others? That looks inconsistent, and relies on an assumption that might
 but does not have to be true (that `int64_t` and `long long` is the same
 type).

 The reason I chose int64_t is that many of us wanted to use 'int64_t'.
 int64_t is 64bit integer.
 size of long long is equal to or larger than size of int64_t.

 The reason I did not use int64_t in some parts is that C++ overloading and
 automatic type conversion do not work well for derived types (int64_t or
 int32_t).
 Overloading function definition side, we cannot define these five
 functions f(int64_t), f(int32_t), f(long long), f(long), f(int) because
 some types are the same sizes.
 Referencing side, f(long long) does not match f(int64_t).

 Then I chose that I defined f(long long), f(long) and f(int).
 Though sizeof(long) may be equal to long long or int (depends on machine
 architecture),
 we can define three functions because they are base types of C++.
 f(int64_t) is linked to f(long long).
 f(int32_t) is lonked to f(int).

 > This comment seems to be for an intermediate version of the code, it is
 no longer true.
 > {{{#!c++
 > //
 > // Type of IntElement is changed from long int to int64_t.
 > // However, strtoint64_t function does not exist.
 > // It is assumed that "long long" and int64_t are the same sizes.
 > // strtoll is used to convert string to integer.
 > //
 > }}}

 Removed it.

 > Is this overloading for all these types necessary? Won't it just auto-
 expand the int type? Again, why is it using `long long` here and not
 `int64_t`?
 > {{{#!c++
 >     bool setValue(const long int i) { return (setValue(static_cast<long
 long int>(i))); };
 >     bool setValue(const int i) { return (setValue(static_cast<long long
 int>(i))); };
 > }}}

 C++ overloading and automatic type conversion do not work well if there
 are string, double and integer.
 On definition side, if we add setValue(int64_t), we cannot add
 setValue(long long), and referencing side, setValue(long long) does not
 match setValue(int64_t).

 > Looking at this change, I wonder if it would be better to use constants
 `LLONG_MAX` and `LLONG_MIN` instead of hardcoding the numbers into the
 test.

 LLONG_MAX is not always equal to 2^31-1 because sizeof(long long) is not
 always 64bit.
 C++ does not have constant of int64_t maximum value.

 > Should the test check the return value (seen at two places):
 > {{{#!c++
 >     EXPECT_NO_THROW(el->intValue());
 > }}}

 I will add.

-- 
Ticket URL: <http://bind10.isc.org/ticket/3015#comment:11>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development