[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-compiler-dev
Subject:    RFR 8189749: Devise strategy for making source level checks more uniform
From:       Maurizio Cimadamore <maurizio.cimadamore () oracle ! com>
Date:       2017-10-20 18:07:05
Message-ID: af46e66f-a0b7-2a65-7ca7-cecd3bbd7a43 () oracle ! com
[Download RAW message or body]

Hi,
this biggie patch allows the treatment of source level checks to be more 
uniform. The general problem is explained in details here:

https://bugs.openjdk.java.net/browse/JDK-8189749

Webrev is here:

http://cr.openjdk.java.net/~mcimadamore/8189749/

The way I approached this, was to create a first class entity, an enum 
called Feature. Each 'feature' has:

* a min source level (e.g. what is the min source level required to 
compile that feature, meaning that lower levels will result in errors)
* a max source level (e.g. what is the max source level required to 
compile that feature, meaning that higher levels will result in errors)
* an optional resource key

The first two properties are obviously used to decided if feature XYZ 
can be used given source level N. The last property is used to automate 
generation of source check diagnostics. Note that this patch introduce a 
_single_ source check diagnostic:

# 0: message segment (feature), 1: string (found version), 2: string 
(expected version)
compiler.misc.feature.not.supported.in.source=\
      {0} not supported in -source {1}\n\
        (use -source {2} or higher to enable {0})

And then there's a bunch of feature fragments, example:

compiler.misc.feature.modules=\
        modules

compiler.misc.feature.diamond.and.anon.class=\
        ''<>'' with anonymous inner classes

compiler.misc.feature.binary.lit=\
        binary literals

(and many more :-))

Since each feature 'knows' its fragment, it is super easy for the 
compiler to generate the source level check diagnostic for any feature. 
And, if you need to add a new feature, you just need to add an enum 
value, and wired it up with the corresponding resource key - done :-)

Since this change affects the way in which source check diagnostics are 
generated, quite a lot of changes were required to adjust expected 
output in golden files. Aside from those changes, I also addressed the 
following issues:

* JavaTokenizer was not using SOURCE_LEVEL flags for its diagnostics, 
and it was not using the new diagnostic framework to generate errors, 
which made the porting harder - so I've fixed that
* CALog in Completeness analyzer required a new override for an 
AbstractLog::error variant that was probably forgotten (this was 
required otherwise CompletenessAnalyzerTest was failing after the change 
above)
* Log has a 'feature' so that diagnostics marked with the SOURCE_LEVEL 
flag can be reported only once per source. But this check was only 
looking at the 'head' of a diagnostic, ignoring all arguments. Since now 
all source level check diagnostics share the same head, I needed to 
tweak the logic to also compare arguments - or else, one source level 
check error being reported would also disable unrelated source level 
check diags.

Then there were a bunch of mechanical translation, for instance idioms 
such as:

allowSimplifiedVarargs = source.allowSimplifiedVarargs();

Were rewritten as:

allowSimplifiedVarargs = source.allowed(Feature.SIMPLIFIED_VARARGS);

Also, in JavacParser, this idiom:

checkTypeAnnotations();

has been replaced with

checkSourceLevel(Feature.TYPE_ANNOTATIONS);

Where checkSourceLevel is a new general routine that works on any Feature.

Finally, where possible, I also got rid of the various boolean flags of 
the kind 'allowPrivateMethodInInterfaces' - if the class already had a 
'source' field, the boolean flag can be omitted, and the check can be 
performed on the source object itself.

Cheers
Maurizio



[prev in list] [next in list] [prev in thread] [next in thread]