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

Tue Jun 2 14:04:25 UTC 2015

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