Re: [EXT] Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Hi Derek,

First can you possibly get your mailer to stop putting [EXT] in the 
subject line because it really messes up threading and filtering.


On 24/05/2019 2:50 am, Derek White wrote:




-Original Message-
From: build-dev  On Behalf Of Volker
Simonis
Sent: Monday, May 20, 2019 2:32 PM
To: Sergey Bylokhov 
Cc: build-dev 
Subject: [EXT] Re: RFR: 8224087: Compile C code for at least C99 Standard
compliance

External Email

--
Sergey Bylokhov  schrieb am Mo. 20. Mai
2019 um
20:14:


Hi, David.
Should not the minimum version of c standard mentioned in the
doc/building.html?



We actually have a Wiki page for that because it’s much easier to maintain:

https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms



Easier for Oracle and SAP apparently 😊.

How do I get write access to the wiki? I can log in but Edit menu items are 
greyed out.


You have to be a member of a Group to edit the Group's wiki content. In 
this case that's the Build Group (which excludes me too).


http://openjdk.java.net/census#build

David


Thanks!
  - Derek

RE: [EXT] Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Derek White]

> -Original Message-
> From: build-dev  On Behalf Of Volker
> Simonis
> Sent: Monday, May 20, 2019 2:32 PM
> To: Sergey Bylokhov 
> Cc: build-dev 
> Subject: [EXT] Re: RFR: 8224087: Compile C code for at least C99 Standard
> compliance
> 
> External Email
> 
> --
> Sergey Bylokhov  schrieb am Mo. 20. Mai
> 2019 um
> 20:14:
> 
> > Hi, David.
> > Should not the minimum version of c standard mentioned in the
> > doc/building.html?
> >
> 
> We actually have a Wiki page for that because it’s much easier to maintain:
> 
> https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms


Easier for Oracle and SAP apparently 😊. 

How do I get write access to the wiki? I can log in but Edit menu items are 
greyed out.

Thanks!
 - Derek

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Thanks Sergey!

David

On 22/05/2019 4:33 am, Sergey Bylokhov wrote:

+1

On 21/05/2019 06:49, Erik Joelsson wrote:

Looks even better!

/Erik

On 2019-05-20 17:56, David Holmes wrote:

Thank you everyone for taking a look at this.

Here is version 2:

http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/

Changes:
- set c99 rather than gnu99
- Volker's change for xlc to match gcc and clang
- added short note to build doc (can do wiki later)
- cosmetic change of name to make variable based on other feedback 
during the C++14 discussion


Thanks,
David

On 20/05/2019 5:40 pm, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 
4.8 annoyed me enough to investigate setting C99 as our minimum 
allow C-language level when compiling. It turned out to be a lot 
more complex a situation than I thought due to toolchain quirks. See 
lots of details in the bug report.


To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather 
than accepting it as default)


I've checked how this works with all the toolchains except xlc as I 
have no access to that. Some assistance from someone who can verify 
the correctness on xlc would be appreciated.


Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Thanks Erik!

David

On 21/05/2019 11:49 pm, Erik Joelsson wrote:

Looks even better!

/Erik

On 2019-05-20 17:56, David Holmes wrote:

Thank you everyone for taking a look at this.

Here is version 2:

http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/

Changes:
- set c99 rather than gnu99
- Volker's change for xlc to match gcc and clang
- added short note to build doc (can do wiki later)
- cosmetic change of name to make variable based on other feedback 
during the C++14 discussion


Thanks,
David

On 20/05/2019 5:40 pm, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 
4.8 annoyed me enough to investigate setting C99 as our minimum allow 
C-language level when compiling. It turned out to be a lot more 
complex a situation than I thought due to toolchain quirks. See lots 
of details in the bug report.


To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than 
accepting it as default)


I've checked how this works with all the toolchains except xlc as I 
have no access to that. Some assistance from someone who can verify 
the correctness on xlc would be appreciated.


Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Hi Thomas,

On 21/05/2019 1:45 pm, Thomas Stüfe wrote:

Hi David,

Thank you for doing this, this looks all very good.


Thanks for taking another look.

I wish though we had a clear whitelist of features to use or blacklist 
for features to avoid. Most developers do not use Windows as a primary 
platform, so it will always be a surprise whether Windows breaks in 
submit tests.


I agree. Unfortunately MS don't even do a good job of documenting their 
supported extensions to C89/90 - AFAICS they don't list for-loop 
declaration expressions.


I am also (a bit) concerned about C99 features creeping in which would 
prevent verbatim backporting of patches to older releases. But let’s see 
if that is really a problem in practice.


Yes lets not try to solve problems that haven't arisen yet. 11u at least 
should be in a position to enforce the same use of C99.


Thanks,
David



Thanks, Thomas


On Tue 21. May 2019 at 02:58, David Holmes > wrote:


Thank you everyone for taking a look at this.

Here is version 2:

http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/

Changes:
- set c99 rather than gnu99
- Volker's change for xlc to match gcc and clang
- added short note to build doc (can do wiki later)
- cosmetic change of name to make variable based on other feedback
during the C++14 discussion

Thanks,
David

On 20/05/2019 5:40 pm, David Holmes wrote:
 > Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
 > webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
 >
 > The need to remove a for-loop declaration expression to appease
gcc 4.8
 > annoyed me enough to investigate setting C99 as our minimum allow
 > C-language level when compiling. It turned out to be a lot more
complex
 > a situation than I thought due to toolchain quirks. See lots of
details
 > in the bug report.
 >
 > To summarise the changes:
 > - gcc: force to -std=gnu99
 > - clang force to -std=gnu99
 > - Solaris studio - no effective change
 > - Visual Studio - no change
 > - xlc - no effective change (but we use the explicit flag rather
than
 > accepting it as default)
 >
 > I've checked how this works with all the toolchains except xlc as
I have
 > no access to that. Some assistance from someone who can verify the
 > correctness on xlc would be appreciated.
 >
 > Thanks,
 > David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Sergey Bylokhov]

+1

On 21/05/2019 06:49, Erik Joelsson wrote:

Looks even better!

/Erik

On 2019-05-20 17:56, David Holmes wrote:

Thank you everyone for taking a look at this.

Here is version 2:

http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/

Changes:
- set c99 rather than gnu99
- Volker's change for xlc to match gcc and clang
- added short note to build doc (can do wiki later)
- cosmetic change of name to make variable based on other feedback during the 
C++14 discussion

Thanks,
David

On 20/05/2019 5:40 pm, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 4.8 annoyed 
me enough to investigate setting C99 as our minimum allow C-language level when 
compiling. It turned out to be a lot more complex a situation than I thought 
due to toolchain quirks. See lots of details in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than accepting 
it as default)

I've checked how this works with all the toolchains except xlc as I have no 
access to that. Some assistance from someone who can verify the correctness on 
xlc would be appreciated.

Thanks,
David



--
Best regards, Sergey.

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Erik Joelsson]

Looks even better!

/Erik

On 2019-05-20 17:56, David Holmes wrote:

Thank you everyone for taking a look at this.

Here is version 2:

http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/

Changes:
- set c99 rather than gnu99
- Volker's change for xlc to match gcc and clang
- added short note to build doc (can do wiki later)
- cosmetic change of name to make variable based on other feedback 
during the C++14 discussion


Thanks,
David

On 20/05/2019 5:40 pm, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 
4.8 annoyed me enough to investigate setting C99 as our minimum allow 
C-language level when compiling. It turned out to be a lot more 
complex a situation than I thought due to toolchain quirks. See lots 
of details in the bug report.


To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than 
accepting it as default)


I've checked how this works with all the toolchains except xlc as I 
have no access to that. Some assistance from someone who can verify 
the correctness on xlc would be appreciated.


Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Thomas Stüfe]

Hi David,

Thank you for doing this, this looks all very good.

I wish though we had a clear whitelist of features to use or blacklist for
features to avoid. Most developers do not use Windows as a primary
platform, so it will always be a surprise whether Windows breaks in submit
tests.

I am also (a bit) concerned about C99 features creeping in which would
prevent verbatim backporting of patches to older releases. But let’s see if
that is really a problem in practice.

Thanks, Thomas


On Tue 21. May 2019 at 02:58, David Holmes  wrote:

> Thank you everyone for taking a look at this.
>
> Here is version 2:
>
> http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/
>
> Changes:
> - set c99 rather than gnu99
> - Volker's change for xlc to match gcc and clang
> - added short note to build doc (can do wiki later)
> - cosmetic change of name to make variable based on other feedback
> during the C++14 discussion
>
> Thanks,
> David
>
> On 20/05/2019 5:40 pm, David Holmes wrote:
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
> > webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
> >
> > The need to remove a for-loop declaration expression to appease gcc 4.8
> > annoyed me enough to investigate setting C99 as our minimum allow
> > C-language level when compiling. It turned out to be a lot more complex
> > a situation than I thought due to toolchain quirks. See lots of details
> > in the bug report.
> >
> > To summarise the changes:
> > - gcc: force to -std=gnu99
> > - clang force to -std=gnu99
> > - Solaris studio - no effective change
> > - Visual Studio - no change
> > - xlc - no effective change (but we use the explicit flag rather than
> > accepting it as default)
> >
> > I've checked how this works with all the toolchains except xlc as I have
> > no access to that. Some assistance from someone who can verify the
> > correctness on xlc would be appreciated.
> >
> > Thanks,
> > David
>

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Thank you everyone for taking a look at this.

Here is version 2:

http://cr.openjdk.java.net/~dholmes/8224087/webrev.v2/

Changes:
- set c99 rather than gnu99
- Volker's change for xlc to match gcc and clang
- added short note to build doc (can do wiki later)
- cosmetic change of name to make variable based on other feedback 
during the C++14 discussion


Thanks,
David

On 20/05/2019 5:40 pm, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 4.8 
annoyed me enough to investigate setting C99 as our minimum allow 
C-language level when compiling. It turned out to be a lot more complex 
a situation than I thought due to toolchain quirks. See lots of details 
in the bug report.


To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than 
accepting it as default)


I've checked how this works with all the toolchains except xlc as I have 
no access to that. Some assistance from someone who can verify the 
correctness on xlc would be appreciated.


Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Hi Dean,

On 21/05/2019 8:59 am, David Holmes wrote:

On 21/05/2019 8:49 am, dean.l...@oracle.com wrote:

Isn't that going to break inline asm?


It doesn't appear to. I'll check what the docs say about such things.


Using c99 means we have to use __asm__ for inline-assembly rather than 
"asm" which is the GNU extension.


Looking at our sources we only use inline assembly in C code in two 
places. The main use is in 
./jdk.crypto.ec/share/native/libsunec/impl/ecl*.c which do use the C99 
__asm__ form.


There is one potential problematic usage in:

./jdk.crypto.ec/share/native/libsunec/impl/mpi.c

but that is ifdef'd on OSF1 which I'm assuming is not enabled in our 
builds anyway.


The other use is

./java.desktop/share/native/libfreetype/src/truetype/ttinterp.c

which is also using the C99 form.

So everything looks fine in that regard.

Thanks again for flagging it.

David
-


Thanks for flagging it.

David
-


dl

On 5/20/19 3:40 PM, Erik Joelsson wrote:
That sounds good to me. I missed that distinction before, but pure 
c99 does sound better.


/Erik

On 2019-05-20 15:31, David Holmes wrote:

Hi Volker,

Thanks for the xlc update - I will incorporate that.

It has been suggested that we actually set c99 rather than gnu99 to 
avoid any GNU extensions creeping in. Does anyone have any concerns 
with doing that?


Thanks,
David

On 21/05/2019 12:05 am, Volker Simonis wrote:

Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html 

[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html 




On Mon, May 20, 2019 at 9:40 AM David Holmes 
 wrote:


Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease 
gcc 4.8

annoyed me enough to investigate setting C99 as our minimum allow
C-language level when compiling. It turned out to be a lot more 
complex
a situation than I thought due to toolchain quirks. See lots of 
details

in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than
accepting it as default)

I've checked how this works with all the toolchains except xlc as 
I have

no access to that. Some assistance from someone who can verify the
correctness on xlc would be appreciated.

Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

On 21/05/2019 8:49 am, dean.l...@oracle.com wrote:

Isn't that going to break inline asm?


It doesn't appear to. I'll check what the docs say about such things.

Thanks for flagging it.

David
-


dl

On 5/20/19 3:40 PM, Erik Joelsson wrote:
That sounds good to me. I missed that distinction before, but pure c99 
does sound better.


/Erik

On 2019-05-20 15:31, David Holmes wrote:

Hi Volker,

Thanks for the xlc update - I will incorporate that.

It has been suggested that we actually set c99 rather than gnu99 to 
avoid any GNU extensions creeping in. Does anyone have any concerns 
with doing that?


Thanks,
David

On 21/05/2019 12:05 am, Volker Simonis wrote:

Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html 

[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html 




On Mon, May 20, 2019 at 9:40 AM David Holmes 
 wrote:


Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 
4.8

annoyed me enough to investigate setting C99 as our minimum allow
C-language level when compiling. It turned out to be a lot more 
complex
a situation than I thought due to toolchain quirks. See lots of 
details

in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than
accepting it as default)

I've checked how this works with all the toolchains except xlc as I 
have

no access to that. Some assistance from someone who can verify the
correctness on xlc would be appreciated.

Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[dean . long]

Isn't that going to break inline asm?

dl

On 5/20/19 3:40 PM, Erik Joelsson wrote:
That sounds good to me. I missed that distinction before, but pure c99 
does sound better.


/Erik

On 2019-05-20 15:31, David Holmes wrote:

Hi Volker,

Thanks for the xlc update - I will incorporate that.

It has been suggested that we actually set c99 rather than gnu99 to 
avoid any GNU extensions creeping in. Does anyone have any concerns 
with doing that?


Thanks,
David

On 21/05/2019 12:05 am, Volker Simonis wrote:

Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html
[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html



On Mon, May 20, 2019 at 9:40 AM David Holmes 
 wrote:


Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 
4.8

annoyed me enough to investigate setting C99 as our minimum allow
C-language level when compiling. It turned out to be a lot more 
complex
a situation than I thought due to toolchain quirks. See lots of 
details

in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than
accepting it as default)

I've checked how this works with all the toolchains except xlc as I 
have

no access to that. Some assistance from someone who can verify the
correctness on xlc would be appreciated.

Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Erik Joelsson]

That sounds good to me. I missed that distinction before, but pure c99 
does sound better.


/Erik

On 2019-05-20 15:31, David Holmes wrote:

Hi Volker,

Thanks for the xlc update - I will incorporate that.

It has been suggested that we actually set c99 rather than gnu99 to 
avoid any GNU extensions creeping in. Does anyone have any concerns 
with doing that?


Thanks,
David

On 21/05/2019 12:05 am, Volker Simonis wrote:

Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html
[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html



On Mon, May 20, 2019 at 9:40 AM David Holmes 
 wrote:


Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 4.8
annoyed me enough to investigate setting C99 as our minimum allow
C-language level when compiling. It turned out to be a lot more complex
a situation than I thought due to toolchain quirks. See lots of details
in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than
accepting it as default)

I've checked how this works with all the toolchains except xlc as I 
have

no access to that. Some assistance from someone who can verify the
correctness on xlc would be appreciated.

Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Hi Volker,

Thanks for the xlc update - I will incorporate that.

It has been suggested that we actually set c99 rather than gnu99 to 
avoid any GNU extensions creeping in. Does anyone have any concerns with 
doing that?


Thanks,
David

On 21/05/2019 12:05 am, Volker Simonis wrote:

Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html
[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html


On Mon, May 20, 2019 at 9:40 AM David Holmes  wrote:


Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 4.8
annoyed me enough to investigate setting C99 as our minimum allow
C-language level when compiling. It turned out to be a lot more complex
a situation than I thought due to toolchain quirks. See lots of details
in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than
accepting it as default)

I've checked how this works with all the toolchains except xlc as I have
no access to that. Some assistance from someone who can verify the
correctness on xlc would be appreciated.

Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Volker Simonis]

Sergey Bylokhov  schrieb am Mo. 20. Mai 2019 um
20:14:

> Hi, David.
> Should not the minimum version of c standard mentioned in the
> doc/building.html?
>

We actually have a Wiki page for that because it’s much easier to maintain:

https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms


> On 20/05/2019 00:40, David Holmes wrote:
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
> > webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
> >
> > The need to remove a for-loop declaration expression to appease gcc 4.8
> annoyed me enough to investigate setting C99 as our minimum allow
> C-language level when compiling. It turned out to be a lot more complex a
> situation than I thought due to toolchain quirks. See lots of details in
> the bug report.
> >
> > To summarise the changes:
> > - gcc: force to -std=gnu99
> > - clang force to -std=gnu99
> > - Solaris studio - no effective change
> > - Visual Studio - no change
> > - xlc - no effective change (but we use the explicit flag rather than
> accepting it as default)
> >
> > I've checked how this works with all the toolchains except xlc as I have
> no access to that. Some assistance from someone who can verify the
> correctness on xlc would be appreciated.
> >
> > Thanks,
> > David
>
>
> --
> Best regards, Sergey.
>

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Sergey Bylokhov]

Hi, David.
Should not the minimum version of c standard mentioned in the doc/building.html?

On 20/05/2019 00:40, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 4.8 annoyed 
me enough to investigate setting C99 as our minimum allow C-language level when 
compiling. It turned out to be a lot more complex a situation than I thought 
due to toolchain quirks. See lots of details in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than accepting 
it as default)

I've checked how this works with all the toolchains except xlc as I have no 
access to that. Some assistance from someone who can verify the correctness on 
xlc would be appreciated.

Thanks,
David



--
Best regards, Sergey.

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Erik Joelsson]

The updated webrev from Volker looks good to me.

Thank you David for fixing this!

/Erik

On 2019-05-20 07:05, Volker Simonis wrote:

Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html
[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html


On Mon, May 20, 2019 at 9:40 AM David Holmes  wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 4.8
annoyed me enough to investigate setting C99 as our minimum allow
C-language level when compiling. It turned out to be a lot more complex
a situation than I thought due to toolchain quirks. See lots of details
in the bug report.

To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than
accepting it as default)

I've checked how this works with all the toolchains except xlc as I have
no access to that. Some assistance from someone who can verify the
correctness on xlc would be appreciated.

Thanks,
David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Volker Simonis]

Hi David,

thanks for considering AIX in your change.

With OpenJDK 13 we've moved to XLC 16 which has a new Clang based
frontend. The "ucs" feature-suboption [1] you're using in your change
is only supported in the old, xlc compiler but not in the new xlclang
any more [2]. However, the good news is that the new xlclang now
supports the same "-std=gn99" option like gcc and clang, so you could
easily handle the xlc case now together with gcc and clang as shown in
this webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8224087/

Otherwise, your change looks good!

Please let me know once you've pushed it. I opened "8224214: [AIX]
Remove support for legacy xlc compiler" [3] for further cleannups
because we still set some "-qlanglvl" options for C++ which aren't
supported by the new compiler either.

Best regards,
Volker

[1] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_12.1.0/com.ibm.xlcpp121.aix.doc/compiler_ref/opt_langlvl.html
[2] 
https://www.ibm.com/support/knowledgecenter/SSGH3R_16.1.0/com.ibm.xlcpp161.aix.doc/compiler_ref/opt_langlvl_aix.html


On Mon, May 20, 2019 at 9:40 AM David Holmes  wrote:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
> webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
>
> The need to remove a for-loop declaration expression to appease gcc 4.8
> annoyed me enough to investigate setting C99 as our minimum allow
> C-language level when compiling. It turned out to be a lot more complex
> a situation than I thought due to toolchain quirks. See lots of details
> in the bug report.
>
> To summarise the changes:
> - gcc: force to -std=gnu99
> - clang force to -std=gnu99
> - Solaris studio - no effective change
> - Visual Studio - no change
> - xlc - no effective change (but we use the explicit flag rather than
> accepting it as default)
>
> I've checked how this works with all the toolchains except xlc as I have
> no access to that. Some assistance from someone who can verify the
> correctness on xlc would be appreciated.
>
> Thanks,
> David

Re: RFR: 8224087: Compile C code for at least C99 Standard compliance

[Andrew Haley]

On 5/20/19 8:40 AM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
> webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/
> 
> The need to remove a for-loop declaration expression to appease gcc 4.8 
> annoyed me enough to investigate setting C99 as our minimum allow 
> C-language level when compiling. It turned out to be a lot more complex 
> a situation than I thought due to toolchain quirks. See lots of details 
> in the bug report.
> 
> To summarise the changes:
> - gcc: force to -std=gnu99
> - clang force to -std=gnu99
> - Solaris studio - no effective change
> - Visual Studio - no change
> - xlc - no effective change (but we use the explicit flag rather than 
> accepting it as default)
> 
> I've checked how this works with all the toolchains except xlc as I have 
> no access to that. Some assistance from someone who can verify the 
> correctness on xlc would be appreciated.

You are a hero for doing this. Thank you, thank you!

Approved by me, checking only Linux targets.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

RFR: 8224087: Compile C code for at least C99 Standard compliance

[David Holmes]

Bug: https://bugs.openjdk.java.net/browse/JDK-8224087
webrev: http://cr.openjdk.java.net/~dholmes/8224087/webrev/

The need to remove a for-loop declaration expression to appease gcc 4.8 
annoyed me enough to investigate setting C99 as our minimum allow 
C-language level when compiling. It turned out to be a lot more complex 
a situation than I thought due to toolchain quirks. See lots of details 
in the bug report.


To summarise the changes:
- gcc: force to -std=gnu99
- clang force to -std=gnu99
- Solaris studio - no effective change
- Visual Studio - no change
- xlc - no effective change (but we use the explicit flag rather than 
accepting it as default)


I've checked how this works with all the toolchains except xlc as I have 
no access to that. Some assistance from someone who can verify the 
correctness on xlc would be appreciated.


Thanks,
David