RFR: JDK-8211148: javac -source 10 in JDK11 is inconsistent with javac in JDK10

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Wed Oct 3 10:18:45 UTC 2018

The changes look generally good, but I have some questions/comments.

* (cosmetic, optional) This change:

if (Feature.VAR_SYNTAX_IMPLICIT_LAMBDAS.allowedInSource(source)) {
+                        log.error(DiagnosticFlag.SYNTAX, param.pos, Errors.VarNotAllowedArray);
+                    } else {
+                        log.error(DiagnosticFlag.SYNTAX, param.pos, Errors.VarNotAllowedHere);
+                    }

Could be slightly simplified as:

log.error(DiagnosticFlag.SYNTAX, param.pos, Feature.VAR_SYNTAX_IMPLICIT_LAMBDAS.allowedInSource(source)
  ? Errors.VarNotAllowedArray : Errors.VarNotAllowedHere);

* A more serious issue is, I think that the code here:

if (Feature.VAR_SYNTAX_IMPLICIT_LAMBDAS.allowedInSource(source)) {
+                        param.startPos = TreeInfo.getStartPos(param.vartype);
+                        param.vartype = null;
+                    } else {
+                        log.error(DiagnosticFlag.SYNTAX, param.pos, Errors.VarNotAllowedHere);
+                    }

Should really call the parser method 'checkSourceLevel' which will issue 
the correct error if the feature is not allowed (e.g. 'var in lambda not 
allowed, use source 11 to enable'). Which brings to:

* You should define a compiler key for the new feature (e.g. 'var in 
lambda') so that the framework will pick that up when generating a 
diagnostic using the above method

* In the classification logic, I believe the goal of the changes is to 
disable the diagnostic generation for illegal combinations of lambda 
parameter declarations when one or more of these involve a 'var' which 
would have already been flagged in a separate diagnostic. That is, these 
changes are to avoid to report duplicate and spurious diagnostics, right?

* In the LambdaParserTest I see no relevant changes, but still the patch 
shows something - is there some trailing space or indent different at play?


On 03/10/18 01:40, Vicente Romero wrote:
> Hi,
> Please review the fix for [1] at [2]. There were a number of places 
> where the compiler was not checking if var syntax for implicit lambdas 
> was allowed or not. This implied that the JDK11 compiler was accepting 
> code with this feature even if the -source 10 option was passed, this 
> shouldn't happen. This patch fixes this and issues the: "var not 
> allowed here" error message as expected.
> Thanks,
> Vicente
> [1] https://bugs.openjdk.java.net/browse/JDK-8211148
> [2] http://cr.openjdk.java.net/~vromero/8211148/webrev.00/jdk.dev.patch

More information about the compiler-dev mailing list