Ignore default methods when scanning interfaces #66
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Namely so that default methods can be used to emulate macros defined in header files we're often replacing when doing ffi work. We must use reflection to call Method.isDefault as it's only available as of Java SE 8 (as are default methods).
Contributor
Author
|
Turns out I tried to catch an exception that's not available in Java 6 and thus the build is broken in my first commit. I don't know what your build-breaking policies are; should I squash the fix in and resubmit? |
Member
|
Sorry for the delay on this. We don't have a strict policy that every commit be buildable, and Java 6 and 7 are largely EOL, so we'll just accept this with the post fix. I'll review. |
headius
approved these changes
Sep 21, 2016
Member
|
I'd like to see a test for this, conditionally on Java 8+, using one of the built-in interfaces with default methods (e.g. java.util.Collection, but perhaps something smaller). Otherwise it looks good to me. |
Member
|
I'll add a test. |
headius
added a commit
that referenced
this pull request
Sep 22, 2016
The test will be skipped on Java 7, but on Java 8 it will ensure that the function mapper does not try to map over top of any default methods in the interface.
goto1134
added a commit
to goto1134/jnr-ffi
that referenced
this pull request
Aug 25, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Namely so that default methods can be used to emulate macros defined in
header files we're often replacing when doing ffi work.
We must use reflection to call Method.isDefault as it's only available
as of Java SE 8 (as are default methods).
Implementing a library's API's macros next to the rest of the ffi declarations seems like a sensible thing to do. Currently, a default method's implementation will be ignored and a link error will result on trying to call the method (if it is indeed only a macro and not an exported symbol). Issues could arise if someone is relying on this behavior, although that would strike me as odd. This could be mitigated with a
@Ignoreannotation on default methods (which would similarly cause the interface scanner to ignore the method), which I'm happy to implement if there's concern.