Hi Kim, could you please take a look at the updated webrev: http://cr.openjdk.java.net/~iignatyev/8144695/webrev.03
I agree that “+w” isn’t related to WARNINGS_ARE_ERRORS, so it was moved to CFLAGS_WARN. Regarding compiler version based conditions, I think it’d be better for build team to decide how to deal w/ them. PS I’ve checked that w/ the patch applied warnings, which normally cause a build error, don’t cause any build errors w/ --disable-warnings-as-errors. Thanks, Igor > On Dec 17, 2015, at 11:30 PM, Kim Barrett <kim.barr...@oracle.com> wrote: > > On Dec 17, 2015, at 8:22 AM, Igor Ignatyev <igor.ignat...@oracle.com> wrote: >> >> >>> On Dec 17, 2015, at 2:10 AM, Kim Barrett <kim.barr...@oracle.com> wrote: >>> make/solaris/makefiles/adlc.make >>> 77 WARNINGS_ARE_ERRORS ?= -w -xwe >>> >>> I'm pretty sure "-w" is wrong here, and should be removed. >> you are right, I made a typo, it was ‘+w’ before. the new webrev : >> http://cr.openjdk.java.net/~iignatyev/8144695/webrev.02/ >> >>> And it's >>> not clear why this assignment should be conditional on the compiler >>> version. >> it was added as a fix for https://bugs.openjdk.java.net/browse/JDK-6851829, >> excerpt from Chris’s evaluation: >> >>> Since some of the errors are in system headers we can only disable the "+w >>> -errwarn" on SS11 and below. > > "+w" has nothing to do with warnings being errors; it just turns on > more warnings. So it shouldn't be in WARNINGS_ARE_ERRORS. > > CFLAGS_WARN is (according to various comments) supposed to hold > options to enable/disable warnings, so "+w" there was reasonable, > while -errwarn should not have been there by that definition. > > The conditionalization disables additional warnings and "warnings are > errors" for older compilers that I think we're no longer using for > jdk9. Are we allowed to retire support for such? > > The conditionalization may only be needed for "+w", though without > testing on a no longer officially supported version of the compiler > that would be hard to prove.
