RFR: JDK-8180744: Update ct.sym for JDK 10

[Maurizio Cimadamore]

Hi Jan,
the patch looks very good. I like the new JDKPlatformProvider and I find 
the new interface a lot cleaner (and I like that you could remove 
support for sig files from other javac components, that is a good sign!).

Overall I think that we should try and improve (not as part of this 
review/bug) CreateSymbols. It is quite a big class, doing many things at 
once. I think at the very least we should try to add more documentations 
(e.g. I found many invariants that are not documented), and, in the long 
run, I think the code would probably benefit if we could split 
CreateSymbols in 3 parts:

* definition of the DescriptionXYZ classes
* one tool to generate the sym.txt files
* one tool to generate ct.sym from description files

Also, I couldn't help but note that certain constraints had a 
significant impact on the implementation complexity:

* the fact that it is not customary to store binary files in openjdk 
probably forced you to 'quote' the classfiles guts (as generated by 
Probe.java)
* a similar restriction was probably the cause for having a text file 
representation rather than a classfile based one (which would have been 
much more amenable to be manipulated with tools such as ASM)
* the desire not to call .sig file 'classfiles' (as they are not 
well-formed) is also causing unnecessary complexity - in principle you 
shouldn't need a special file manager to load those

Anyway, all the comments are clearly outside the scope of this review - 
I just hope that at some point some of these decisions would be looked 
at again, to see if we can achieve this feature in a way that's easier 
to maintain in the long run.

Cheers
Maurizio


On 13/10/17 13:30, Jan Lahoda wrote:
> Hi,
>
> The patch here adds a support for --release 9 to OpenJDK. This 
> includes adding a snapshot of the JDK 9 APIs.
>
> Notes:
> -several changes to the historical data in make/data/symbols:
> --java.management.rmi-8.sym.txt contains a few classes that were 
> originally in java.management-8.sym.txt (this change is adjusting the 
> structure to adhere more to the final JDK 9 module layout)
> --java.annotations.common-* renamed to java.xml.ws.annotation-* to 
> adhere to the final layout
> --diffing of classes across releases has been improved to avoid some 
> unnecessary class header notices in the historical data
> --empty files are now not written for the historical data
> -the (JDK)PlatformProvider.PlatformDescription(Impl) now returns a 
> file manager, instead of a list of paths. This makes the contract 
> cleaner, and allow to handle the ".sig" extension mostly in the file 
> manager instead of ClassFinder. (Due to this change, JDK-8139607: 
> '-release option forces StandardJavaFileManager' is also resolved by 
> this patch, although it is not the primary goal of this patch.)
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8180744
> Webrev: http://cr.openjdk.java.net/~jlahoda/8180744/webrev.00/
>
> I'll send to build-dev as well after the javac changes will look OK.
>
> Any feedback is welcome.
>
> Thanks,
>     Jan