On Fri, 4 Nov 2022 12:38:04 GMT, Adam Sotona <asot...@openjdk.org> wrote:
> This is root pull request with Classfile API implementation, tests and > benchmarks initial drop into JDK. > > Following pull requests consolidating JDK class files parsing, generating, > and transforming ([JDK-8294957](https://bugs.openjdk.org/browse/JDK-8294957)) > will chain to this one. > > Classfile API development is tracked at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch > > Development branch of consolidated JDK class files parsing, generating, and > transforming is at: > https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch > > Classfile API [JEP](https://bugs.openjdk.org/browse/JDK-8280389) and [online > API > documentation](https://htmlpreview.github.io/?https://raw.githubusercontent.com/openjdk/jdk-sandbox/classfile-api-javadoc-branch/doc/classfile-api/javadoc/jdk/classfile/package-summary.html) > is also available. > > Please take you time to review this non-trivial JDK addition. > > Thank you, > Adam make/CompileInterimLangtools.gmk line 78: > 76: $(call LogInfo, Generating Preview.java for jdk.compiler.interim) > 77: $(call MakeDir, $(@D)) > 78: $(GREP) -v 'case PATTERN_SWITCH ->' $< > $@ I understand that you have based this on the example above. There is a huge difference, though. The sed expression just updates the package name to include the new `interim` part. These new additions seems to be designed to filter out actual code. Since no reason for this is given in comments, I'm assuming there is some code that either does not compile when building this for the interim JDK, or that gives the incorrect result if included. But that means that now suddenly the makefiles has intricate knowledge about specific lines of code in the source code! That is not a good entanglement to have. make/CompileInterimLangtools.gmk line 84: > 82: $(call LogInfo, Generating TransPatterns.java for jdk.compiler.interim) > 83: $(call MakeDir, $(@D)) > 84: $(GREP) -v 'Assert.check(preview.' $< > $@ (The same goes for this instance, of course) make/modules/java.base/Java.gmk line 37: > 35: > 36: EXCLUDES += java/lang/doc-files > 37: EXCLUDES += jdk/classfile/snippets I don't like hard-coded excludes like this. Is this the first real-world example of snippets in the JDK? If not, how has this been resolved in other instances? Maybe we need a more general method to exclude snippets from compiling. make/test/BuildMicrobenchmark.gmk line 106: > 104: --add-exports java.base/jdk.classfile.components=ALL-UNNAMED \ > 105: --add-exports java.base/jdk.classfile.impl=ALL-UNNAMED \ > 106: --add-exports > java.base/jdk.internal.org.objectweb.asm=ALL-UNNAMED \ Why do you need to export ASM now, when it was not needed before? I thought the purpose here was to replace ASM..? ------------- PR: https://git.openjdk.org/jdk/pull/10982
