Request for review: 7102489: RFE: cleanup jlong typedef on __APPLE__ and _LP64 systems

[David Holmes]

Hi Harold,

On 12/01/2013 1:02 AM, harold seigel wrote:
> Thanks for your comments. My replies are interspersed.
>
> Harold
>
> On 1/11/2013 2:33 AM, David Holmes wrote:
>>
>> On 11/01/2013 4:39 AM, harold seigel wrote:
>>> Hi,
>>>
>>> Please review the following changes to fix bug 7102489.
>>>
>>> Summary:
>>> The definition of type jlong differed on Mac OS from the other 64 bit
>>> platforms. This fix makes it consistent. In order to do this, this fix
>>> defines new macros, JLONG_FORMAT and JULONG_FORMAT, for printing and
>>> scanning jlongs and julongs.
>>>
>>> This fix also does some cleanup. Methods jlong_format_specifier() and
>>> julong_format_specifer() were removed and some format specifiers were
>>> replaced with appropriate macros.
>>
>> General clean up looks good. As long as we have jlong and int64 types
>> in use I much prefer to JLONG_FORMAT and INT64_FORMAT in the sources.
>>
>> I still wonder how those os:: functions came about in the first place :)
>
> Coleen looked into this but they date from long ago.
>>
>> A few comments ...
>>
>> src/os/posix/launcher/java_md.h
>>
>> 67 #define JLONG_FORMAT "%lld"
>>
>> Doesn't this need to have both 32-bit and 64-bit variants?
>>
>> --
>>
>> src/os/windows/launcher/java_md.h
>>
>> + #define JLONG_FORMAT "%I64d"
>>
>> Ditto ?
>
> These changes just replace calls to jlong_format_specifier() with a
> JLONG_FORMAT macro that has the same value. Since there wasn't a need
> for 32-bit and 64-bit variants for jlong_format_specifier(), I don't
> think variants were needed for JLONG_FORMAT.

I understand you did a simple replacement but it seems to me the 
original code should have had different versions for 32-bit and 64-bit.

>> --
>>
>> src/share/vm/utilities/globalDefinitions_gcc.hpp
>>
>> Why do we need a special case for Apple? AFAIK for ILP32 and LP64
>> "long long" is always 64-bits so %lld should always be valid.
>>
> This answer is similar to the one I sent to Vladimir. We need a special
> case for Apple because its compiler is picky about format specifiers. It
> will issue a warning for values of type long that have format specifiers
> of %lld.

Understood, but why is that occurring? If we always have

typedef long jlong => JLONG_FORMAT == %ld
typedef long long jlong =>  JLONG_FORMAT == %lld

where are things out of sync on Apple?

David
-----

For example, on Mac OS, this program gets a compilation warning
> at line 6, but not line 5.
>
> $ gcc lld.cpp
> lld.cpp: In function int main(int, char**):
> lld.cpp:6: warning: format %lld expects type long long int, but argument
> 2 has type long int
>
> $ cat lld.cpp
> #include <stdio.h>
>
> int main(int argc, char * argv[]) {
> long val = 5;
> printf("val: %ld\n", val);
> printf("val: %lld\n", val); <--- line 6
> return 0;
> }
>> --
>>
>> src/share/vm/utilities/ostream.cpp
>>
>> I think the N.B comments could be deleted.
>
> I agree. I'll delete those comments.
>
>>
>> --
>>
>> Thanks,
>> David
>>
>>
>>> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_7102489/
>>> <http://cr.openjdk.java.net/%7Ehseigel/bug_7102489/>
>>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=7102489
>>>
>>> Thank you,
>>> Harold