JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version

Wed May 27 09:59:38 UTC 2015

Looks great. The only nitpick is that the comments in CreateSymbols 
don't mention the fact that a side effect of the sym.txt generation is 
the  <platform-description-file> mentioned earlier in the same comments 
(so one might wonder where does that come from).

Maurizio

On 27/05/15 10:37, Jan Lahoda wrote:
> Hi,
>
> I've uploaded another iteration, with these changes:
> -the "symbols" file is now generated automatically (for the typical 
> OpenJDK case).
> -fixed a minor issue in CreateSymbols that could cause putting class 
> description into wrong a file (the "break" -> "break OUTER;" change).
> -jdk.management module has been split out from java.management 
> recently, so splitting the corresponding .sym.txt files into 
> java.management and jdk.management as well.
> -updating the copyright year in CreateSymbols, as noted by Magnus.
>
> Webrevs:
> -top-level:
> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.02/top-level/
> -langtools (no changes against the last webrev):
> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.02/langtools/
>
> Delta webrevs against the previous iteration are included under 
> "Author comments".
>
> Thanks for the comments so far!
>
> Jan
>
> On 22.5.2015 15:22, Jan Lahoda wrote:
>> On 22.5.2015 14:52, Maurizio Cimadamore wrote:
>>> Excellent work.
>>>
>>> I think the comment in CreateSymbols could be made clearer w.r.t. Probe
>>> - i.e. that Probe should be ran on top of the JDK N - i.e.
>>>
>>> <JDK-8>/bin/java Probe --> classes-8
>>> <JDK-7>/bin/java Probe --> classes-7
>>> <JDK-6>/bin/java Probe --> classes-7
>>>
>>> etc.
>>
>> Sure, will do.
>>
>> I'll also look at generating the make/data/symbols/symbols descriptions
>> automatically, per our offline discussion.
>>
>> Thanks!
>>
>> Jan
>>
>>>
>>> Maurizio
>>>
>>> On 22/05/15 13:38, Jan Lahoda wrote:
>>>> Hi,
>>>>
>>>> I've uploaded a new iteration of the patch(es):
>>>> top-level repository:
>>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.01/top-level/
>>>> langtools:
>>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.01/langtools/
>>>>
>>>> (besides full webrevs, there are also webrevs showing the differences
>>>> between .00 and .01 available - see the "Delta webrev" link under
>>>> "Author's comments")
>>>>
>>>> Summary of changes:
>>>> -applied Eric's build changes
>>>> -moved CreateSymbols into make/src/classes/build/tools/symbolgenerator
>>>> -tried to improve the specification of base platforms in
>>>> CreateSymbols, per Maurizio's comment
>>>> -other cleanup in langtools per Maurizio's comments.
>>>>
>>>> Thanks,
>>>>     Jan
>>>>
>>>> On 21.5.2015 12:31, Maurizio Cimadamore wrote:
>>>>> Hi Jan,
>>>>> great work - couple of comments below:
>>>>>
>>>>> * it seems like some of the routines in Arguments can be simplified
>>>>> using lambdas - especially lookupPlatformProvider and
>>>>> checkOptionAllowed
>>>>> * Why JDKPlatformProvider is not called 
>>>>> JDKPlatformProvider*Factory* ?
>>>>> * JavacProcessingEnvironment.JoiningIterator seems to have
>>>>> commonalities
>>>>> with CompoundScopeIterator - any chance that we might refactor this a
>>>>> bit?
>>>>> * What's the rationale for giving an error if -platform is specified
>>>>> and
>>>>> a non-StandardFileManager is provided? Can't we simply swallow that,
>>>>> ignore the platform settings and issue a Lint 'options' warning?
>>>>> * Would it make sense for some of the classfile generation logic in
>>>>> CreateSymbols to be moved under the classfile API ?
>>>>> * I had hard time reconciling some of the comments in CreateSymbols
>>>>> with
>>>>> how the files were laid out. I think in the end, what you mean is 
>>>>> that
>>>>> if you have platforms 7, 8, 9 - you should pick one baseline (i.e. 8)
>>>>> and then generate diffs for 9 and 7 against the 8 one. If that's the
>>>>> use
>>>>> case, I think the command line can be simplified a bit in order to
>>>>> explicitly state which of the platform is the baseline one, and the
>>>>> remaining parameters can be inferred from the tool (as the
>>>>> <base-platform-for-platform1,2 ... N> seem to be typically all
>>>>> identical
>>>>> but one which is set to <none> - the one for the baseline). Maybe the
>>>>> inference logic should be different (i.e. use 8 as a baseline for 
>>>>> 7 and
>>>>> 7 as a baseline for 6) - but - whatever the logic, I think it 
>>>>> should be
>>>>> chosen once and for all, and live implicitly in the tool? Or are 
>>>>> there
>>>>> reasons as to why it might be handy to customize the baselines?
>>>>>
>>>>> Maurizio
>>>>>
>>>>> On 21/05/15 08:01, Jan Lahoda wrote:
>>>>>> Hi,
>>>>>>
>>>>>> This is a patch adding a new option, -platform, to javac.
>>>>>>
>>>>>> Patch for the top-level repository is here:
>>>>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.00/top-level/
>>>>>>
>>>>>> Patch for the langtools repository is here:
>>>>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.00/langtools/
>>>>>>
>>>>>> These changes also require additional langtools changes as their
>>>>>> prerequisite:
>>>>>> http://cr.openjdk.java.net/~jlahoda/8080675/webrev.00/
>>>>>>
>>>>>> Overall, one can imagine '-platform N' to have the same effect as
>>>>>> '-source N -target N -bootclasspath <APIs-for-N>'. The possible 
>>>>>> values
>>>>>> for 'N' are pluggable in a limited way, though (see
>>>>>> langtools/src/jdk.compiler/share/classes/com/sun/tools/javac/platform/PlatformProvider.java). 
>>>>>>
>>>>>>
>>>>>>
>>>>>> Note that this patch only introduces N=7 and N=8, which 
>>>>>> correspond to
>>>>>> Open JDK 7 and 8 GA.
>>>>>>
>>>>>> A tricky problem to solve is where to get the APIs for platform 
>>>>>> N. The
>>>>>> proposal is to include history data in the textual format inside the
>>>>>> OpenJDK repositories (the input data), process them at build time 
>>>>>> and
>>>>>> create a lib/ct.sym file holding the data in the JDK image. javac
>>>>>> running with the -platform option then compiles against the 
>>>>>> lib/ct.sym
>>>>>> file. The input history data are placed
>>>>>> <top-level-repository>/make/data/symbols (the sym.txt files). This
>>>>>> patches only includes data for OpenJDK 7 and 8, and only for public
>>>>>> APIs (more or less Java SE and JDK @Exported APIs).
>>>>>>
>>>>>> The size of the history data is currently ~6MB in the JDK 
>>>>>> checkout and
>>>>>> ~650kB inside the .hg directory (the size could change significantly
>>>>>> if additional classes/elements, like non-public API classes, would
>>>>>> need to be included). The lib/ct.sym file is currently ~4.5MB.
>>>>>>
>>>>>> The ct.sym file is a zip file containing signature files. The
>>>>>> signature files are similar to classfiles (so javac can read them as
>>>>>> classfiles), but are missing some attributes, most notably the Code
>>>>>> attribute. On the top-level, the ct.sym contains directories named
>>>>>> "7", "78" and "8". When compiling for version 'N', the bootclasspath
>>>>>> for that version is obtained by using directories which have 'N' in
>>>>>> their name.
>>>>>>
>>>>>> The sigfiles for ct.sym are created by
>>>>>> <top-level-repository>/make/tools/symbolgenerator/CreateSymbols.java. 
>>>>>>
>>>>>> The same file can also be used to construct the sym.txt files. 
>>>>>> Concise
>>>>>> instructions are part of the CreateSymbols.java.
>>>>>>
>>>>>> I am sending this as one review, as the changes together make one
>>>>>> feature, but the langtools and top-level changes are independent 
>>>>>> to a
>>>>>> great degree: the langtools changes add the "-platform" javac; 
>>>>>> and the
>>>>>> top-level changes add the history data and ability to build the 
>>>>>> ct.sym
>>>>>> file. If desired, these could be pushed and/or reviewed 
>>>>>> independently.
>>>>>>
>>>>>> Many thanks go to Jon Gibbons, Joe Darcy, Magnus Ihse Bursie, Alan
>>>>>> Bateman and others who have provided advices and help on the matter
>>>>>> before.
>>>>>>
>>>>>> Any insights/comments are wholeheartedly welcome.
>>>>>>
>>>>>> Thanks,
>>>>>>     Jan
>>>>>
>>>