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

Fri Jun 26 09:49:49 UTC 2015

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
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>
>>>>
>