[9] RFR(L): 8037816 : Fix for 8036122 breaks build with Xcode5/clang
David Holmes
david.holmes at oracle.com
Tue Apr 22 01:57:49 UTC 2014
On 18/04/2014 9:31 AM, David Chase wrote:
>
> On 2014-04-17, at 6:26 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>
>> + #if defined(__clang_major__) || (__GNUC__ >= 4) && (__GNUC_MINOR__ >= 6)
>> Do all versions of Clang support push and pop?
>
> I don't know -- what versions of clang are we likely to use?
>
>>> (3) In many cases, the best fix is to replace a call to a method expecting a format with one to a method expecting only a string. That is, replace “print(s)” with “print_raw(s)”. Note that when there is a single parameter this cannot possibly be a mistake.
>>
>> Yes, it fixes the problem but it will be difficult to remember or know (for new people) that you should use print_raw instead.
>
> What's the alternative? Once we get our compilers up to decent revisions, the checking will just be there, and
> anyone who writes print(s) will get an error.
>
> The reason I used "print_raw" was that it was already there, and it was the least-violence change to the code;
> no need to insert "%s", before the string, etc. "raw" makes it sound sort of unsafe, but it's the moral equivalent of puts(s).
Sorry but I much prefer putting the %s before the string and keeping a
nice clean simple API. I don't know the history of the existing raw
usage but this broad change looks very ugly to me - and given you had to
change the call-sites anyway, it seems the addition of "%s" is the more
correct fix.
Cheers,
David H.
>
>>> (4) Use new P2I macro to clean up some of the Linux format warnings; this was feasible for some of the printf-like internal methods that were not called too often with technically-mismatched types and format specifiers.
>>> #define P2I(p) ((intptr_t)(p))
>>
>> Can we please not use a macro for this? Use an inline method instead.
>
> Will do. What does a top-level inline method look like?
> Would this do?
>
> static inline intptr_t p2i(void * p) { return (intptr_t) p; }
>
> David
>
More information about the hotspot-dev
mailing list