On 8/24/16, 5:31 AM, Yasumasa Suenaga wrote: > Hi Erik, Phil, > > Thank you for replying. > I understand background of JDK-8074827. > >> In this particular case is shift-negative-value a new warning in GCC 6? > > Yes, this feature is implemented GCC 6: > https://gnu.wildebeest.org/blog/mjw/2016/02/15/looking-forward-to-gcc6-many-new-warnings/
I see. I tracked down the commit so yes, this seems to be the case. > > BTW, why is libjavajpeg only enabled these warnings? > For example, liblcms is disabled format-nonliteral, type-limits, and > misleading-indentation. We only disable the warnings that are observed .. not all warnings else we might as well turn off warnings completely. > > I agree compiler warnings is very useful to fix. However, I think a > part of source of libjavajpeg is third-party (developed by Independent > JPEG Group). > According to [1], warnings in this library should be suppressed. Yes. FWIW this is the most stable 3rd party library we have - by a long way - so it *could* be argued that tweaking it isn't likely to get lost any time soon but I'd still prefer to keep the source as it came. > > If all binaries which are included in JDK/JRE should be enabled all > compiler warnings, I think LCMS and any other libraries should be fixed. Well you would *have* to get upstream to resolve it. But it is whack-a-mole. As you have discovered new compilers generate new warnings .. and it is not monotonic either. -phil. > > Which policy is correct? > > > Thanks, > > Yasumasa > > > [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-June/004495.html > > > On 2016/08/24 18:48, Erik Joelsson wrote: >> Hello, >> >> >> On 2016-08-23 18:12, Phil Race wrote: >>> On 08/23/2016 08:47 AM, Erik Joelsson wrote: >>>> Hello, >>>> >>>> I do agree that maintaining the list of disabled warnings will be >>>> impossible unless we have a structured way of tracking for which >>>> compiler versions we disable what. Ideally we should be able to easily >>>> add conditions for when certain warnings should be disabled. We are >>>> unfortunately lacking that today and at least I don't have the >>>> bandwidth to fix that anytime soon. >>>> >>>> The official compilers are only really official for Oracle. The >>>> OpenJDK can (and should) be buildable with a much wider range of >>>> compiler versions. >>> I agree there. This is fortunately not an "unbuildable" situation. >>> The only other option I can think of which may or may not be palatable >>> is to explicitly >>> check the compiler version and add that disabled warning only for that >>> exact compiler version. >>> There'd still be some maintenance as that compiler version became >>> either >>> official .. or obsolete .. >>> >>> Is there precedent (or any kind of support) for that ? >> What I had in mind was a structured way of adding conditionals for >> some kind of ranges of compiler versions, or at least something like >> 6.*, or "greater than 4.9.3". It's pretty simple today to check for >> exact compiler versions but then we end up with a lot of changes when >> minor versions are bumped. I don't think that would be worth it. >> >> In this particular case is shift-negative-value a new warning in GCC >> 6? If that's the case it doesn't actually hurt adding it since GCC is >> nice enough to not complain about unknown warning tags. If we do, >> just make sure to specify in a comment that it's specific to GCC >> version 6+. >> >> /Erik >>> -phil. >>> >>>> Luckily we have the workaround of setting --disable-warnings-as-errors >>>> when this situation occurs. >>>> >>>> For reference, the compilers Oracle uses are listed in >>>> README-builds.md in the top level directory in the forest. >>>> >>>> So for now at least, I think Phil is right. >>>> >>>> /Erik >>>> >>>> On 2016-08-23 17:11, Philip Race wrote: >>>>> Erik .. please chime in if you disagree with the following >>>>> The goal here is to have no warnings with the *official* compilers. >>>>> If you are using something else and get warnings then either fix >>>>> the *source* or else you need to use --disable-warning-as-errors. >>>>> >>>>> Otherwise we'll be suppressing the warnings for a whole range >>>>> of compilers and no one will know what the set is and these >>>>> bugs will become 'unfixable' and a continual chore of churn. >>>>> It will be something we should address as we move *official* >>>>> compilers. >>>>> >>>>> In fact the warning you want to suppress is one I just got rid of >>>>> (the >>>>> suppression) >>>>> http://hg.openjdk.java.net/jdk9/client/jdk/rev/d4f7412a51d2 >>>>> I don't think we want to add it back unless the *official* compilers >>>>> object >>>>> .. I am sure that official list is documented somewhere. >>>>> >>>>> -phil. >>>>> >>>>> >>>>> On 8/23/16, 6:10 AM, Yasumasa Suenaga wrote: >>>>>> Hi all, >>>>>> >>>>>> I've fixed several warnings when we build OpenJDK with GCC 6 in >>>>>> JDK-8160294. >>>>>> However, I encountered shift-negative-value warnings at jdhuff.c >>>>>> on my >>>>>> Fedora 24 (GCC 6.1.1): >>>>>> -------------- >>>>>> /home/ysuenaga/OpenJDK/hs/jdk/src/java.desktop/share/native/libjavajpeg/jdhuff.c:458:13: >>>>>> >>>>>> >>>>>> warning: left shift of negative value [-Wshift-negative-value] >>>>>> { 0, ((-1)<<1) + 1, ((-1)<<2) + 1, ((-1)<<3) + 1, ((-1)<<4) + 1, >>>>>> ^~ >>>>>> /home/ysuenaga/OpenJDK/hs/jdk/src/java.desktop/share/native/libjavajpeg/jdhuff.c:458:28: >>>>>> >>>>>> >>>>>> warning: left shift of negative value [-Wshift-negative-value] >>>>>> { 0, ((-1)<<1) + 1, ((-1)<<2) + 1, ((-1)<<3) + 1, ((-1)<<4) + 1, >>>>>> ^~ >>>>>> /home/ysuenaga/OpenJDK/hs/jdk/src/java.desktop/share/native/libjavajpeg/jdhuff.c:458:43: >>>>>> >>>>>> >>>>>> warning: left shift of negative value [-Wshift-negative-value] >>>>>> { 0, ((-1)<<1) + 1, ((-1)<<2) + 1, ((-1)<<3) + 1, ((-1)<<4) + 1, >>>>>> : >>>>>> -------------- >>>>>> >>>>>> I think these warnings are available from JDK-8074827. >>>>>> This issue enables warnings to fix in the source code. >>>>>> However, in review of JDK-8160294, I heared that warnings in IJG >>>>>> JPEG >>>>>> library should just be suppressed: >>>>>> >>>>>> http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-June/004497.html >>>>>> >>>>>> >>>>>> >>>>>> So I think they should be suppressed and we should fix as below: >>>>>> -------------- >>>>>> diff -r b548b8217d8c make/lib/Awt2dLibraries.gmk >>>>>> --- a/make/lib/Awt2dLibraries.gmk Mon Aug 22 19:28:36 2016 -0700 >>>>>> +++ b/make/lib/Awt2dLibraries.gmk Tue Aug 23 22:08:44 2016 +0900 >>>>>> @@ -490,7 +490,7 @@ >>>>>> CFLAGS := $(CFLAGS_JDKLIB) $(BUILD_LIBJAVAJPEG_HEADERS) \ >>>>>> $(LIBJAVA_HEADER_FLAGS) \ >>>>>> -I$(SUPPORT_OUTPUTDIR)/headers/java.desktop, \ >>>>>> - DISABLED_WARNINGS_gcc := clobbered, \ >>>>>> + DISABLED_WARNINGS_gcc := clobbered shift-negative-value, \ >>>>>> MAPFILE := $(JDK_TOPDIR)/make/mapfiles/libjpeg/mapfile-vers, \ >>>>>> LDFLAGS := $(LDFLAGS_JDKLIB) \ >>>>>> $(call SET_SHARED_LIBRARY_ORIGIN), \ >>>>>> -------------- >>>>>> >>>>>> If it is correct, I file it to JBS and upload webrev. >>>>>> Do you think about it? >>>>>> >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Yasumasa >>>>>> >>>>>> >>
