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

Tue Jun 2 16:50:52 UTC 2015

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