Skip to content

Conversation

mcimadamore
Copy link
Collaborator

@mcimadamore mcimadamore commented Sep 8, 2025

This change adds a new sample of an annotation processor that uses code models to:

  • reject unsupported methods (e.g. System.gc, System.exit)
  • reject unsupported language construct (e.g. try, throw)

The annotation processor is somewhat configurable, and can be used by developers as an initial sketch upon which to build more custom compile-time checks.

When using the annotation processor to compile this:

class Foo {
   @CodeReflection
   void test1() {
      System.exit(1);
   }

    @CodeReflection
    void test2() {
        System.gc();
    }

    @CodeReflection
    void test3() {

       try {
           test2();
       } finally {
           test3();
       }

    }
}

The following error messages are generated:

Foo.java:5: error: System.exit not supported in reflectable methods
   void test1() {
        ^
Foo.java:10: error: System.gc not supported in reflectable methods
    void test2() {
         ^
Foo.java:15: error: try/catch statement not supported in reflectable methods
    void test3() {
         ^

When putting this together, I noted some issues, reported below:

  • javac was failing to execute annotation processors that depended on jdk.incubator.code module
  • obtaining the model from a method that contains attribution errors crashed the annotation processor
  • some packages (e.g. the ones in java.base) are not exported to the dynamic module layer's jdk.incubator.code
  • it is not possible to report the message at a more accurate location because the Messager API does not allow for this

Many thanks to @lahodaj for the invaluable help when fighting with module layers :-)


Progress

  • Change must not contain extraneous whitespace

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/babylon.git pull/554/head:pull/554
$ git checkout pull/554

Update a local copy of the PR:
$ git checkout pull/554
$ git pull https://git.openjdk.org/babylon.git pull/554/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 554

View PR using the GUI difftool:
$ git pr show -t 554

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/babylon/pull/554.diff

Using Webrev

Link to Webrev Comment

@bridgekeeper
Copy link

bridgekeeper bot commented Sep 8, 2025

👋 Welcome back mcimadamore! A progress list of the required criteria for merging this PR into code-reflection will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk
Copy link

openjdk bot commented Sep 8, 2025

@mcimadamore This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

Add sample annotation processor using code models

Reviewed-by: psandoz

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the code-reflection branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the code-reflection branch, type /integrate in a new comment.

@@ -191,7 +193,9 @@ public void clear() {
}

protected ClassLoader getClassLoader(URL[] urls) {
ClassLoader thisClassLoader = getClass().getClassLoader();
ClassLoader thisClassLoader = CodeReflectionSupport.CODE_LAYER != null ?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to use a "base" classloader which is the same as the one used to build the jdk.incubator.code module layer (if it exists), otherwise we won't be able to load types defined there.

@@ -1169,6 +1171,8 @@ public <S> ServiceLoader<S> getServiceLoader(Location location, Class<S> service
Configuration cf = bootLayer.configuration().resolveAndBind(ModuleFinder.of(), finder, Collections.emptySet());
ModuleLayer layer = bootLayer.defineModulesWithOneLoader(cf, ClassLoader.getSystemClassLoader());
return ServiceLoader.load(layer, service);
} else if (CodeReflectionSupport.CODE_LAYER != null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In some cases javac uses the service loader API to load processors and plugins directly -- again, we need to make sure the jdk.incubator.code layer is used if present.

attribBlock -> {
try {
return reflectMethods.getMethodBody(enclosingClass, methodTree, attribBlock, make);
} catch (Throwable ex) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This avoids crashing if reflectMethods throws (which can happen if the source code we come from contains type checking errors -- such as calling methods with wrong arity)


@SupportedAnnotationTypes("jdk.incubator.code.CodeReflection")
@SupportedSourceVersion(SourceVersion.RELEASE_26)
public class CodeReflectionProcessor extends AbstractProcessor {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we agree this example is sufficient, I can add some documentation to describe what it does, and how to use it

@@ -1169,6 +1171,8 @@ public <S> ServiceLoader<S> getServiceLoader(Location location, Class<S> service
Configuration cf = bootLayer.configuration().resolveAndBind(ModuleFinder.of(), finder, Collections.emptySet());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect something like this:

            ModuleLayer augmentedModuleLayer;
            ClassLoader parentCL;
            if (CodeReflectionSupport.CODE_LAYER != null) {
                augmentedModuleLayer = CodeReflectionSupport.CODE_LAYER;
                parentCL = CodeReflectionSupport.CODE_LAYER.findLoader("jdk.incubator.code");
            } else {
                augmentedModuleLayer = bootLayer;
                parentCL = ClassLoader.getSystemClassLoader();
            }
            Configuration cf = augmentedModuleLayer.configuration().resolveAndBind(ModuleFinder.of(), finder, Collections.emptySet());
            ModuleLayer layer = augmentedModuleLayer.defineModulesWithOneLoader(cf, parentCL);
            return ServiceLoader.load(layer, service);

here rather than the CODE_LAYER != null test below.

Make sure java.base packages are exported to dynamic layer's module
// We also need to add exports all java.base packages so that the plugin can use them
// But we need to do so by calling a method in java.base reflectively
try {
Class<?> codeModuleLayerInit = Class.forName("jdk.internal.access.code.CodeModuleLayerInit");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the babylon module is not in the module graph when running javac, then we need to explicitly export all java.base packages to that module, to make sure it runs correctly (this might be too broad).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note - we have to call through reflection, otherwise javac might fail to compile against a bootstrap JDK

@mcimadamore mcimadamore marked this pull request as ready for review September 10, 2025 11:07
@openjdk openjdk bot added ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 10, 2025
@mlbridge
Copy link

mlbridge bot commented Sep 10, 2025

Webrevs

@openjdk
Copy link

openjdk bot commented Sep 12, 2025

@mcimadamore this pull request can not be integrated into code-reflection due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout codemodel_anno_processor
git fetch https://git.openjdk.org/babylon.git code-reflection
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge code-reflection"
git push

@openjdk
Copy link

openjdk bot commented Sep 12, 2025

⚠️ @mcimadamore This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

@openjdk openjdk bot added merge-conflict Pull request has merge conflict with target branch and removed ready Pull request is ready to be integrated labels Sep 12, 2025
@mcimadamore mcimadamore force-pushed the codemodel_anno_processor branch from 58e493e to 2e5767f Compare September 12, 2025 16:17
@openjdk
Copy link

openjdk bot commented Sep 12, 2025

@mcimadamore Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

@mcimadamore
Copy link
Collaborator Author

/integrate

@openjdk openjdk bot added ready Pull request is ready to be integrated and removed merge-conflict Pull request has merge conflict with target branch labels Sep 12, 2025
@openjdk
Copy link

openjdk bot commented Sep 12, 2025

Going to push as commit c163836.

@openjdk openjdk bot added the integrated Pull request has been integrated label Sep 12, 2025
@openjdk openjdk bot closed this Sep 12, 2025
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Sep 12, 2025
@openjdk
Copy link

openjdk bot commented Sep 12, 2025

@mcimadamore Pushed as commit c163836.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrated Pull request has been integrated
Development

Successfully merging this pull request may close these issues.

3 participants