Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 li
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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'
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
This looks good to me now. /Erik On 2015-06-02 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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 yo
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 pla
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 2015-06-02 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! From a build perspective, this looks good - nah, awesome! :) - to me. /Magnus 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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 d
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 seem to
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 seem to be typically all identical but one which is set to - 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://c
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 seem to be typically all identical but one which is set to - 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 '. 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 inclu
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 seem to be typically all identical but one which is set to - 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 '. 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), pro
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 seem to be typically all identical but one which is set to - 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 '. 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 /make/data/symbols (the sym.txt files). This patches only includes data for OpenJDK 7 and 8, and
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 2015-05-22 14: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. Looks good to me! Copyright year in CreateSymbols.java says 2014, maybe time to update it? :-) /Magnus 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 seem to be typically all identical but one which is set to - 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 '. 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 /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 /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 abili
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /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 seem to be typically all identical but one which is set to - 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 '. 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 /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 /make/tools/symbolgenerator/CreateSymbols.java. The same file can also be used to constru
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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. /bin/java Probe --> classes-8 /bin/java Probe --> classes-7 /bin/java Probe --> classes-7 etc. 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 seem to be typically all identical but one which is set to - 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 '. 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 /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 /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 c
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 seem to be typically all identical but one which is set to - 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 '. 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 /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 /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 Bursi
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 22.5.2015 08:49, Erik Joelsson wrote: On 2015-05-21 21:59, mark.reinh...@oracle.com wrote: 2015/5/21 12:01 -0700, jan.lah...@oracle.com: 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/ Nice work -- I'm glad to see this coming in. What's the rationale for putting the symbol generator and all its data in the top-level repo? That'll add quite a bit of heft to a repo that's always been intended to remain fairly lightweight. Wouldn't it make more sense for this code and data to be placed in the langtools repo? Or is there some sort of build-bootstrapping issue here? Technically these files could go in any repo, it's only a question of where they best fit from a logical/maintaining point of view. I had not thought about the langtools repo before, I just thought that it didn't fit in the jdk repo so better put it in the top as it applies to all modules. But the user of this data is the compiler so langtools does make sense. There has been a brief discussion on which repo to use some time ago, and top-level repo seemed reasonable. I am personally fine with the langtools repo, if that's preferred. (Langtools repo is often cloned and used standalone, so these standalone checkouts would be bigger, but probably not a big problem.) Thanks, Jan /Erik
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 2015-05-21 21:59, mark.reinh...@oracle.com wrote: 2015/5/21 12:01 -0700, jan.lah...@oracle.com: 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/ Nice work -- I'm glad to see this coming in. What's the rationale for putting the symbol generator and all its data in the top-level repo? That'll add quite a bit of heft to a repo that's always been intended to remain fairly lightweight. Wouldn't it make more sense for this code and data to be placed in the langtools repo? Or is there some sort of build-bootstrapping issue here? Technically these files could go in any repo, it's only a question of where they best fit from a logical/maintaining point of view. I had not thought about the langtools repo before, I just thought that it didn't fit in the jdk repo so better put it in the top as it applies to all modules. But the user of this data is the compiler so langtools does make sense. /Erik
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
2015/5/21 12:01 -0700, jan.lah...@oracle.com: > 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/ Nice work -- I'm glad to see this coming in. What's the rationale for putting the symbol generator and all its data in the top-level repo? That'll add quite a bit of heft to a repo that's always been intended to remain fairly lightweight. Wouldn't it make more sense for this code and data to be placed in the langtools repo? Or is there some sort of build-bootstrapping issue here? - Mark
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 21/05/15 14:14, Jan Lahoda wrote: For 6, 7, 8, ... we could assume there is an ordering. But seems to me that the flexibility of being able to specify the baseline (rather than having the chosen automatically based on the version number) is not bad. But I can change it to automatic baseline per the design above, if you prefer. I guess it would be nice (as discussed privately) if a side-effect of this effort was also to specify which diff files are generated and how given a set of available platforms. Maurizio
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 21.5.2015 14:14, Maurizio Cimadamore wrote: On 21/05/15 12:48, Jan Lahoda wrote: As an example, consider we would be currently storing data for 6, 7 and 8. We could have full 8 APIs stored, and then 8->7 diff and 7->6 diff. So the baseline for 7 would be 8 and the baseline for 6 would be 7. When the data for 9 would be added(*), we could keep the full APIs for 8 (to avoid wasting space in the repository), and then store 8->9 and 8->7 diffs (and drop 6). So 8 would be the baseline for both 7 and 9. So, some flexibility may be useful here. Does this make some sense? It seems to me that versions form a total order. If you pick N to be your baseline, you should generate a full API for that K, and then generate incremental diffs for K < N and K > N - example: case 1: platforms: { 6, 7, 8 } baseline = 8 files: 8, 8->7, 7->6 case 2: platforms { 7, 8, 9 } baseline = 8 files: 8->9, 8, 8->7 So, can't we just assume that there's a set of platforms (which can be sorted), and a baseline K pointing at one of them? Then it's easy to figure out how you should generate diffs: sym(N) := , where N == K sym(N) := diff between sym(N-1) and N, where N > K sym(N) := diff between N and sym(N + 1), where N < K For 6, 7, 8, ... we could assume there is an ordering. But seems to me that the flexibility of being able to specify the baseline (rather than having the chosen automatically based on the version number) is not bad. But I can change it to automatic baseline per the design above, if you prefer. Thanks, Jan Maurizio
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 21/05/15 12:48, Jan Lahoda wrote: As an example, consider we would be currently storing data for 6, 7 and 8. We could have full 8 APIs stored, and then 8->7 diff and 7->6 diff. So the baseline for 7 would be 8 and the baseline for 6 would be 7. When the data for 9 would be added(*), we could keep the full APIs for 8 (to avoid wasting space in the repository), and then store 8->9 and 8->7 diffs (and drop 6). So 8 would be the baseline for both 7 and 9. So, some flexibility may be useful here. Does this make some sense? It seems to me that versions form a total order. If you pick N to be your baseline, you should generate a full API for that K, and then generate incremental diffs for K < N and K > N - example: case 1: platforms: { 6, 7, 8 } baseline = 8 files: 8, 8->7, 7->6 case 2: platforms { 7, 8, 9 } baseline = 8 files: 8->9, 8, 8->7 So, can't we just assume that there's a set of platforms (which can be sorted), and a baseline K pointing at one of them? Then it's easy to figure out how you should generate diffs: sym(N) := , where N == K sym(N) := diff between sym(N-1) and N, where N > K sym(N) := diff between N and sym(N + 1), where N < K Maurizio
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
Hi Magnus, On 21.5.2015 13:21, Magnus Ihse Bursie wrote: Hi Erik and Jan, Erik's modifications look much better to me. I'm just not entirerly satisfied with how this new symbolgenerator tool fits int our model. It's really a buildtool, similar to the other buildtools we have. First of all, the location is not really ideal. Compare with how the java build tools are stored, e.g. jdk/make/src/classes/build/tools/charsetmapping. Correspondingly, I believe the symbolgenerator should live in $TOP/make/src/classes/build/tools/symbolgenerator. This makes it clear that it's source code, leaves room for native build tools, and sets up a common package prefix (while build.tools. might not be ideal, it's the same as in the JDK build tools, so let's keep it). Sure - I'll move it. Thanks, Jan Second, and this is more of a open question, mostly directed at Erik, shouldn't we build the symbolgenerator tool at the buildtools phase? It might require slightly more work to adapt the buildtools system for the top level, but I believe it is much more in line with what we got. /Magnus On 2015-05-21 11:42, Erik Joelsson wrote: Hello Jan, On the build changes there are some things I would like to change. * The creation of the ct.sym file should be done in a separate file, with a separate top level target. The images target should just be about linking and pulling things together, not actual building/compiling of things. * The calls to MakeDir are not needed. * There are missing prerequisites in the rule for creating the symbols. * The jar file creation should use the SetupArchive macro. * Intermediate build files should be put into the $(SUPPORT_OUTPUTDIR). * I think it makes sense to have the ct.sym file be part of the exploded-image. Since this was quite a lot of changes, I took the liberty of implementing them. Here is a webrev with my suggestion for build changes: http://cr.openjdk.java.net/~erikj/8072480/webrev.01/ /Erik On 2015-05-21 09: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 '. 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 /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 /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
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
On 2015-05-21 13:21, Magnus Ihse Bursie wrote: Hi Erik and Jan, Erik's modifications look much better to me. I'm just not entirerly satisfied with how this new symbolgenerator tool fits int our model. It's really a buildtool, similar to the other buildtools we have. First of all, the location is not really ideal. Compare with how the java build tools are stored, e.g. jdk/make/src/classes/build/tools/charsetmapping. Correspondingly, I believe the symbolgenerator should live in $TOP/make/src/classes/build/tools/symbolgenerator. This makes it clear that it's source code, leaves room for native build tools, and sets up a common package prefix (while build.tools. might not be ideal, it's the same as in the JDK build tools, so let's keep it). I agree with this. Second, and this is more of a open question, mostly directed at Erik, shouldn't we build the symbolgenerator tool at the buildtools phase? It might require slightly more work to adapt the buildtools system for the top level, but I believe it is much more in line with what we got. We had an offline discussion and concluded that the build tools building needs to be redone in some better way, for this new tool and for all the existing ones. Until this happens, we will accept the current solution here. /Erik /Magnus On 2015-05-21 11:42, Erik Joelsson wrote: Hello Jan, On the build changes there are some things I would like to change. * The creation of the ct.sym file should be done in a separate file, with a separate top level target. The images target should just be about linking and pulling things together, not actual building/compiling of things. * The calls to MakeDir are not needed. * There are missing prerequisites in the rule for creating the symbols. * The jar file creation should use the SetupArchive macro. * Intermediate build files should be put into the $(SUPPORT_OUTPUTDIR). * I think it makes sense to have the ct.sym file be part of the exploded-image. Since this was quite a lot of changes, I took the liberty of implementing them. Here is a webrev with my suggestion for build changes: http://cr.openjdk.java.net/~erikj/8072480/webrev.01/ /Erik On 2015-05-21 09: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 '. 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 /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 /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 in
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
Hi Maurizio, Thanks for the comments. 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 Yes, I'll take a look. * Why JDKPlatformProvider is not called JDKPlatformProvider*Factory* ? A remnant of history, I'll fix that. * JavacProcessingEnvironment.JoiningIterator seems to have commonalities with CompoundScopeIterator - any chance that we might refactor this a bit? I'll take a look. * 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? If the bootclasspath cannot be set for -platform N, javac would be compiling against the current version of the API, not against the N's APIs. And so the result may not be able to run on N. So it seemed more appropriate to me to refuse to continue rather than continue and produce a potentially wrong result. * Would it make sense for some of the classfile generation logic in CreateSymbols to be moved under the classfile API ? Possibly - the Classfile API could be made more friendly to create classes from scratch. I'd prefer to keep that separate from this effort, though. * 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 Ok - I'll work on making the comments more comprehensible. 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 seem to be typically all identical but one which is set to - 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? As an example, consider we would be currently storing data for 6, 7 and 8. We could have full 8 APIs stored, and then 8->7 diff and 7->6 diff. So the baseline for 7 would be 8 and the baseline for 6 would be 7. When the data for 9 would be added(*), we could keep the full APIs for 8 (to avoid wasting space in the repository), and then store 8->9 and 8->7 diffs (and drop 6). So 8 would be the baseline for both 7 and 9. So, some flexibility may be useful here. Does this make some sense? (*) For JDK 9, there are no history data stored for 9, -platform 9 uses the real JDK 9's bootclasspath (for JDK 9, -platform 9 is mostly a no-op). We would presumably add the history data for 9 when JDK 9 is finished. Thanks! Jan 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 '. 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 /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 c
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
Hi Erik and Jan, Erik's modifications look much better to me. I'm just not entirerly satisfied with how this new symbolgenerator tool fits int our model. It's really a buildtool, similar to the other buildtools we have. First of all, the location is not really ideal. Compare with how the java build tools are stored, e.g. jdk/make/src/classes/build/tools/charsetmapping. Correspondingly, I believe the symbolgenerator should live in $TOP/make/src/classes/build/tools/symbolgenerator. This makes it clear that it's source code, leaves room for native build tools, and sets up a common package prefix (while build.tools. might not be ideal, it's the same as in the JDK build tools, so let's keep it). Second, and this is more of a open question, mostly directed at Erik, shouldn't we build the symbolgenerator tool at the buildtools phase? It might require slightly more work to adapt the buildtools system for the top level, but I believe it is much more in line with what we got. /Magnus On 2015-05-21 11:42, Erik Joelsson wrote: Hello Jan, On the build changes there are some things I would like to change. * The creation of the ct.sym file should be done in a separate file, with a separate top level target. The images target should just be about linking and pulling things together, not actual building/compiling of things. * The calls to MakeDir are not needed. * There are missing prerequisites in the rule for creating the symbols. * The jar file creation should use the SetupArchive macro. * Intermediate build files should be put into the $(SUPPORT_OUTPUTDIR). * I think it makes sense to have the ct.sym file be part of the exploded-image. Since this was quite a lot of changes, I took the liberty of implementing them. Here is a webrev with my suggestion for build changes: http://cr.openjdk.java.net/~erikj/8072480/webrev.01/ /Erik On 2015-05-21 09: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 '. 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 /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 /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
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 seem to be typically all identical but one which is set to - 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 '. 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 /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 /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
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
Hello Eric, Thanks a lot for your feedback and changes! I'll fold them into the patch. Thanks, Jan On 21.5.2015 11:42, Erik Joelsson wrote: Hello Jan, On the build changes there are some things I would like to change. * The creation of the ct.sym file should be done in a separate file, with a separate top level target. The images target should just be about linking and pulling things together, not actual building/compiling of things. * The calls to MakeDir are not needed. * There are missing prerequisites in the rule for creating the symbols. * The jar file creation should use the SetupArchive macro. * Intermediate build files should be put into the $(SUPPORT_OUTPUTDIR). * I think it makes sense to have the ct.sym file be part of the exploded-image. Since this was quite a lot of changes, I took the liberty of implementing them. Here is a webrev with my suggestion for build changes: http://cr.openjdk.java.net/~erikj/8072480/webrev.01/ /Erik On 2015-05-21 09: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 '. 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 /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 /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
Re: JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
Hello Jan, On the build changes there are some things I would like to change. * The creation of the ct.sym file should be done in a separate file, with a separate top level target. The images target should just be about linking and pulling things together, not actual building/compiling of things. * The calls to MakeDir are not needed. * There are missing prerequisites in the rule for creating the symbols. * The jar file creation should use the SetupArchive macro. * Intermediate build files should be put into the $(SUPPORT_OUTPUTDIR). * I think it makes sense to have the ct.sym file be part of the exploded-image. Since this was quite a lot of changes, I took the liberty of implementing them. Here is a webrev with my suggestion for build changes: http://cr.openjdk.java.net/~erikj/8072480/webrev.01/ /Erik On 2015-05-21 09: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 '. 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 /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 /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
JDK 9 RFR of JDK-8072480: javac should support compilation for a specific platform version
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 '. 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 /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 /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