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

Fri Jun 26 13:20:34 UTC 2015

Based on additional Jon's feedback, I made some minor tweaks to the 
patch. An updated webrev is here:
http://cr.openjdk.java.net/~jlahoda/8072480/webrev.06/langtools/

A delta webrev is available under "Author comments".

Unless there are objections, I'll work on pushing this to jdk9/dev.

Thanks for all the comments!

Jan

On 26.6.2015 11:49, Maurizio Cimadamore wrote:
> Looks good to me too; I like the name choice of the new
> Provider/Description pair.
>
> Maurizio
>
> On 25/06/15 22:32, Jonathan Gibbons wrote:
>> Looks good to me :-)
>>
>> -- Jon
>>
>> On 06/24/2015 06:13 AM, Jan Lahoda wrote:
>>> Hello,
>>>
>>> After some offline discussions, I've somewhat changed the internal
>>> API for plugging in the platforms (based on Jon's advices). An
>>> updated webrev is here:
>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.05/langtools/
>>>
>>> How does this look?
>>>
>>> Thanks for all the comments!
>>>
>>> Jan
>>>
>>> On 2.6.2015 18:50, Jan Lahoda wrote:
>>>> Hello Eric,
>>>>
>>>> Thanks for the change, this seems definitely better to me. I've folded
>>>> your change that into my patch. An updated version (just langtools this
>>>> time):
>>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.04/langtools/
>>>>
>>>> Thanks!
>>>>
>>>> Jan
>>>>
>>>> On 2.6.2015 16:04, Erik Joelsson wrote:
>>>>> Hello Jan,
>>>>>
>>>>> Sorry to bother you with even more build changes, but with these file
>>>>> moves, I realized that this new file, ct.sym, is really a part of the
>>>>> jdk.compiler module and really not a special case at all. Because of
>>>>> this, it should be generated as part of the jdk.compiler-gendata
>>>>> target.
>>>>> This also eliminates all the changes in the top level repo and only
>>>>> leaves one new makefile in the langtools repo.
>>>>>
>>>>> http://cr.openjdk.java.net/~erikj/8072480/webrev.02/
>>>>>
>>>>> /Erik
>>>>>
>>>>> On 2015-06-01 17:56, Jan Lahoda wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I made a somewhat bigger update (partially based on other feedback).
>>>>>> Summary of changes:
>>>>>> -the history data has been moved into langtools (I also moved the
>>>>>> Ctsym.gmk)
>>>>>> -there are JDK 6 data now
>>>>>> -renamed the "-platform" option to "-release". Code/tests directly
>>>>>> related to the option has been also changed as well. I kept the
>>>>>> internal PlatformProvider API in javac as is, and also kept related
>>>>>> code.
>>>>>> -added a note that the <platform-description-file> is generated by
>>>>>> CreateSymbols.
>>>>>>
>>>>>> Webrevs:
>>>>>> top-level:
>>>>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.03/top-level/
>>>>>> langtools:
>>>>>> http://cr.openjdk.java.net/~jlahoda/8072480/webrev.03/langtools/
>>>>>>
>>>>>> Delta webrevs are also available.
>>>>>>
>>>>>> How does this look?
>>>>>>
>>>>>> Thanks for the comments so far!
>>>>>>
>>>>>> Jan
>>>>>>
>>>>>> On 27.5.2015 14:23, Jan Lahoda wrote:
>>>>>>> Ah, yes, I did not realize that. Thanks, will fix.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>      Jan
>>>>>>>
>>>>>>> On 27.5.2015 11:59, Maurizio Cimadamore wrote:
>>>>>>>> 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
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>
>>>>>
>>
>