Hi Dalibor,

Thank you for the response! I agree with you, if there are no others 
complaining about this behavior change, I would not care. In fact, the new 
behavior is more correct, although I am not sure, why previous compilers did 
not detect this bug in our code. I think it must have something to do with the 
additional if statement in the finally block that somehow prevents the compiler 
from correctly analyzing the use of final fields. In fact the rule is quite 
simple: the final field must be assigned before it is first used. The 
try/finally combination does not guarantee that the field is assigned when the 
finally block runs.

I tried the same with another code variant without the "if" and it failed to 
compile in Java 7 and earlier Java 8, too. I have the feeling this change was a 
side effect of some other compiler cleanup.

It might be good to put a hint in the release notes that there might be a 
different, but more correct behavior, in compiling code that uses final fields 
in ctors. Unfortunately, I have no idea what combinations of final field 
assignments worked before. I only know, that incorrect use was detected 
correctly in most cases.

-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: u...@thetaphi.de


> -----Original Message-----
> From: Dalibor Topic [mailto:dalibor.to...@oracle.com]
> Sent: Saturday, June 07, 2014 12:29 PM
> To: rory.odonn...@oracle.com; Uwe Schindler
> Cc: Balchandra Vaidya
> Subject: Re: Possible regression in 8u20 build 15
> 
> On 5/26/14 12:20 PM, Rory O'Donnell Oracle, Dublin ireland wrote:
> > Hi Uwe,
> >
> > cc'ing Dalibor.
> 
> Yeah, I agree with Uwe's analysis - the compiler got smarter in this case. I'm
> not sure if this is technically a regression - looking at
> https://bugs.openjdk.java.net/browse/JDK-7004835 and similar issues in JBS
> around handling of finals in constructors, it looks like the compiler is 
> doing the
> right thing now.
> 
> If you strongly feel that this is a change potentially impacting many 
> projects,
> you can of course file an issue, and we'll see if we need to revisit this 
> change
> again. My understanding from the Lucene issue is that this is the only
> occurrence you've seen on the Lucene code base, though, and I haven't seen
> other reports of the problem - so it may be a rare issue in practice.
> 
> cheers,
> dalibor topic
> 
> > Rgds, Rory
> >
> > On 05/24/14 03:13 PM, Uwe Schindler wrote:
> >>
> >> Hi Rory,
> >>
> >>
> >>
> >> we found a possible regression in build 15 of 8u20. The javac compiler
> (correctly) reports a bug in our Lucene code, but earlier versions never
> complained. We fixed the bug in our code, see
> https://issues.apache.org/jira/browse/LUCENE-5704, but this may affect
> other properties. This “bug” does not really affect us, because our code can
> handle “null” correctly. But the new compiler is right.
> >>
> >>
> >>
> >> I am not sure if this is a regression or not. The compiler behavior changed
> a bit. The new behavior is correct, but as this was not detected as syntax
> error before, it might break other projects, too.
> >>
> >>
> >>
> >> This code failed to compile (compiles fine in 8u5 and all Java 7 releases) 
> >> –
> this is simplified, not a complete test case, just shows the problem:
> >>
> >>
> >>
> >> final IndexInput data;
> >>
> >> public MyClass() {
> >>
> >>     success = false;
> >>
> >>     try {
> >>
> >>       String dataName =
> IndexFileNames.segmentFileName(state.segmentInfo.name,
> state.segmentSuffix, dataExtension);
> >>
> >>       this.data = state.directory.openInput(dataName, state.context);
> >>
> >>       final int version2 = CodecUtil.checkHeader(data, dataCodec,
> >>
> >>                                                  VERSION_START,
> >>
> >>                                                  VERSION_CURRENT);
> >>
> >>       if (version != version2) {
> >>
> >>         throw new CorruptIndexException("Format versions mismatch");
> >>
> >>       }
> >>
> >>
> >>
> >>       success = true;
> >>
> >>     } finally {
> >>
> >>       if (!success) {
> >>
> >>         IOUtils.closeWhileHandlingException(this.data);
> >>
> >>       }
> >>
> >>     }
> >>
> >> }
> >>
> >>
> >>
> >> To make it compile, you have to move the first 2 lines of the try block
> outside:
> >>
> >>
> >>
> >> final IndexInput data;
> >>
> >> public MyClass() {
> >>
> >>     success = false;
> >>
> >>     String dataName =
> IndexFileNames.segmentFileName(state.segmentInfo.name,
> state.segmentSuffix, dataExtension);
> >>
> >>     this.data = state.directory.openInput(dataName, state.context);
> >>
> >>     try {
> >>
> >>       final int version2 = CodecUtil.checkHeader(data, dataCodec,
> >>
> >>                                                  VERSION_START,
> >>
> >>                                                  VERSION_CURRENT);
> >>
> >>       if (version != version2) {
> >>
> >>         throw new CorruptIndexException("Format versions mismatch");
> >>
> >>       }
> >>
> >>
> >>
> >>       success = true;
> >>
> >>     } finally {
> >>
> >>       if (!success) {
> >>
> >>         IOUtils.closeWhileHandlingException(this.data);
> >>
> >>       }
> >>
> >>     }
> >>
> >> }
> >>
> >>
> >>
> >> This is of course the correct behaviour!
> >>
> >>
> >>
> >> Uwe
> >>
> >>
> >>
> >> -----
> >>
> >> Uwe Schindler
> >>
> >> H.-H.-Meier-Allee 63, D-28213 Bremen
> >>
> >> http://www.thetaphi.de
> >>
> >> eMail: u...@thetaphi.de
> >>
> >>
> >>
> >>
> >>
> >
> > --
> > Rgds,
> > Rory O'Donnell
> >
> > Senior Quality Engineering Manager
> > Java Platform Group
> > Oracle EMEA , Block P5,
> > East Point Business Park, Dublin 3
> > Phone: +353 (0)1 8033887
> >
> 
> 
> --
> Oracle <http://www.oracle.com>
> Dalibor Topic | Principal Product Manager
> Phone: +494089091214 <tel:+494089091214> | Mobile: +491737185961
> <tel:+491737185961>
> Oracle Java Platform Group
> 
> ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg
> 
> ORACLE Deutschland B.V. & Co. KG
> Hauptverwaltung: Riesstr. 25, D-80992 München
> Registergericht: Amtsgericht München, HRA 95603
> Geschäftsführer: Jürgen Kunz
> 
> Komplementärin: ORACLE Deutschland Verwaltung B.V.
> Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
> Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
> Geschäftsführer: Alexander van der Ven, Astrid Kepper, Val Maher
> 
> Green Oracle <http://www.oracle.com/commitment> Oracle is committed
> to developing practices and products that help protect the environment


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to