Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Primitives for Module support #168

Closed
Pante opened this issue Nov 11, 2022 · 7 comments · Fixed by #169
Closed

[FEATURE REQUEST] Primitives for Module support #168

Pante opened this issue Nov 11, 2022 · 7 comments · Fixed by #169
Assignees
Labels
area: elementary enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@Pante
Copy link
Owner

Pante commented Nov 11, 2022

Is your feature request related to a problem? Please describe.
This issue discusses how to add low-level support for modules. This excludes anything in com.elementary.junit.*. High-level/user-facing changes is tracked in #147.

At the moment, Compiler and DaemonCompiler does not look at the module-path while compiling files. This causes files to be missing from compilation when using the @Introspect annotation in a project that uses modules.

At the moment Compiler has no concept of modules. All it does is accept a list of files and outputs the results of compiling said files. We need some mechanism of traversing the modules, and adding all files in those modules. However, how will this affect performance? Traversing all modules that the current module relies on doesn't sound cheap.

In the initial prototype, we accessed the classes by getting the module of the test class and subsequently traversing through them:

    public static DaemonCompiler of(Class<?> annotated) {
        var module = annotated.getModule();
        if (module.isNamed()) {
            var modules = module.getLayer().modules();
            // traverse modules and add to classpath
        }
        return of(scan(annotated));
    }

If done correctly, we might not need to expose annotations to use modules since it's done automatically.

@Pante Pante added enhancement New feature or request help wanted Extra attention is needed area: elementary labels Nov 11, 2022
@Pante Pante added this to the 2.0.0 milestone Nov 11, 2022
@Pante Pante self-assigned this Nov 11, 2022
@CC007
Copy link
Contributor

CC007 commented Nov 11, 2022

What happens if the current module isn't named (aka a project without module-info.java), but it does rely on classes inside a named module? How does that behave and do any changes need to be made to support that?

@Pante
Copy link
Owner Author

Pante commented Nov 12, 2022

What happens if the current module isn't named (aka a project without module-info.java), but it does rely on classes inside a named module? How does that behave and do any changes need to be made to support that?

From my testing, the current set-up already handles non-module code calling module code. The classes will be available on the class-path. The only case that’s not currently supported is module code to module code.

@Pante
Copy link
Owner Author

Pante commented Nov 12, 2022

My main concern is how we’re going to detect modules for @inline and @classpath. Or is it even necessary to do so in those cases?

@CC007
Copy link
Contributor

CC007 commented Nov 12, 2022

My main concern is how we’re going to detect modules for @inline and @classpath. Or is it even necessary to do so in those cases?

If those pose a problem, maybe modules could be added more explicitly

@Pante
Copy link
Owner Author

Pante commented Nov 12, 2022

My main concern is how we’re going to detect modules for @inline and @classpath. Or is it even necessary to do so in those cases?

If those pose a problem, maybe modules could be added more explicitly

I thought about it and it shouldn't matter. Thinking about it, not specifying a module when compiling @Inline and @Classpath will cause it to be treated as pre-JDK9 code which is fine.

The only concern is if they're compiling a module-info with that. However I believe that to be rare enough of an edge case that I won't bother with it.

@Pante Pante linked a pull request Nov 12, 2022 that will close this issue
@CC007
Copy link
Contributor

CC007 commented Nov 13, 2022

My main concern is how we’re going to detect modules for @inline and @classpath. Or is it even necessary to do so in those cases?

If those pose a problem, maybe modules could be added more explicitly

I thought about it and it shouldn't matter. Thinking about it, not specifying a module when compiling @Inline and @Classpath will cause it to be treated as pre-JDK9 code which is fine.

The only concern is if they're compiling a module-info with that. However I believe that to be rare enough of an edge case that I won't bother with it.

Well... I was debating using that, since I will eventually need to test my @NotNullByDefault and @NullableByDefault annotations for modules. It would be hard to test both of those using @Introspect and I don't even know how I would even target those using that annotation, so I think that I'm going to need @Inline for those tests.

@Pante
Copy link
Owner Author

Pante commented Nov 13, 2022

I think the issue in general is that there's no good way to retrieve the class information from an inline source since it's not yet compiled. I'm honestly not sure how to handle such a use-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: elementary enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants