Review for 7141242: build-infra merge: Rename CPP->CXX and LINK->LD

David Holmes david.holmes at oracle.com
Thu Feb 2 17:43:15 PST 2012


Hi Erik,

I think this has gone as far as it needs for now. My visual inspection 
of these changes looks okay.

My lingering concern is the impact on external scripts etc that may set 
some of the renamed flags. Even I have a build script that sets things 
so that I don't get complaints about using the wrong compiler version on 
Solaris.

David

On 2/02/2012 7:59 PM, Erik Joelsson wrote:
> Hello David,
>
> Thanks for taking a look!
>
> New webrev here: http://cr.openjdk.java.net/~erikj/7141242/webrev.01/
> JPRT job running.
>
> In this version a lot more has changes, see comments inline.
>
> On 2012-02-02 03:33, David Holmes wrote:
>> Hi Erik,
>>
>> Lots of CCC to CXX too :)
>>
> Right, it looked to me like CCC was used in rules.make by someone who
> didn't like using CPP for the C++ compiler. I couldn't see any need for
> an intermediate variable there, just extra confusion.
>
>> One compatibility concern: anyone currently setting CPP_FLAGS or
>> LINK_FLAGS etc, externally, will need to change to the new names.
>> Probably worth sending a wider email (jdk8-dev?) when this gets pushed.
>>
> Good point. We will need to send it out both to jdk8 and jdk7 consumers
> as this will (unfortunately) also hit 7u4.
>> make/bsd/makefiles/gcc.make
>>
>> - CPP = $(CXX)
>> + CXX = $(CXX)
> Thanks for spotting that. Fixed in new webrev. I think I've created
> variations on this patch too many times now.
>>
>> C++ flags passed to C compiler?
>>
> That looks weird yes. I don't dare changing it in the scope of this work
> though.
>> make/*/makefiles/rules.make
>>
>> -# $(CC) is the c compiler (cc/gcc), $(CCC) is the c++ compiler (CC/g++).
>> -C_COMPILE = $(CC) $(CPPFLAGS) $(CFLAGS)
>> -CC_COMPILE = $(CCC) $(CPPFLAGS) $(CFLAGS)
>> +# $(CC) is the c compiler (cc/gcc), $(CXX) is the c++ compiler (CC/g++).
>> +C_COMPILE = $(CC) $(CXXFLAGS) $(CFLAGS)
>> +CC_COMPILE = $(CXX) $(CXXFLAGS) $(CFLAGS)
>>
>> The original code is confusing, given that CC is the C compiler it
>> makes no sense that a C++ compile be called CC_COMPILE. Is it worth
>> changing these to CC_COMPILE and CXX_COMPILE? Maybe a secondary cleanup?
>>
> Either a secondary cleanup or all at once. The new webrev deals with
> these and the related COMPILE.CC. These changes aren't needed for
> build-infra but they sure make the code clearer. Basically:
>
> CC_COMPILE -> CXX_COMPILE
> C_COMPILE -> CC_COMPILE
> *.CC -> *.CXX
> *.c -> *.CC
> Removed *.cpp as they weren't used
> (* is COMPILE, GENASM, LINK, LINK_LIB and PREPROCESS)
>
> Question is, how far do we want to go? With these changes, we have
> consistent naming of CC and CXX in all cases that I have found.
>
>> You missed a couple of scripts on Windows that use LINK_VER:
>>
>> windows/get_msc_ver.sh
>> windows/build_vm_def.sh
> I skipped the scripts as it didn't seem needed for my purposes, but
> included them in the new webrev.
>
> /Erik


More information about the hotspot-runtime-dev mailing list