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

Fix #2012: Fix not reachable definitions of default methods #2040

Merged

Conversation

WojciechMazur
Copy link
Contributor

This PR resolves #2012. Default method implementations were up until now not used in the reachability check process.

This issue was also observed #2022, #2023

@WojciechMazur WojciechMazur changed the title Fix not reachable usages of default methods Fix not reachable definitions of default methods Nov 27, 2020
@WojciechMazur WojciechMazur marked this pull request as draft November 27, 2020 18:46
@WojciechMazur
Copy link
Contributor Author

WojciechMazur commented Nov 27, 2020

I've probably run of iteration on a version other than 2.11.x which led me to the wrong conclusions. The bug still exists.
I'm converting this to draft as I'm going to fix it anyway it the next days.

@WojciechMazur WojciechMazur marked this pull request as ready for review November 27, 2020 19:56
@WojciechMazur
Copy link
Contributor Author

WojciechMazur commented Nov 27, 2020

There is a single error still existing, but it disappears when java.util.Iterator.remove() is marked as JavaDefaultMethod.

can't call D6removeuEO on @"T82org.scalanative.testsuite.javalib.lang.IterableDefaultTest$$anon$3$$anon$6$$anon$7"

The iterator in this test does not override the remove() method what results in the mentioned error. Personally, I would say that is expected behavior.

I propose to merge this PR, followed by merging #2004 when it's ready.

@LeeTibbert
Copy link
Contributor

Wojciech,

LGTM!

Thank you for doing this work. It is a lot smaller, tighter, and elegant than the NirGenStat.scala work that I about
to submit this week. I learned a lot of compiler internals, so the time was well spent.

I tested (test-all, which includes scripted) with my zoo of files having JavaDefaultMethods (Map.scala,
Iterator.scala, Comparator.scala, Predicate.scala, Function.scala, Iterable.scala) with both Scala 2.11.12
and Scala 2.13.4. All passed. Looking good....

If it is agreeable to you, I will wait for this PR to be merged, and then rebase PR 2004 and, upon passing,
promote it out of WIP.

Once that PR builds, I will start rolling though the rest of my JavaDefaultMethod related PRs in the queue,
rebasing, rebuilding, & promoting.

Sound like a plan? Please advise if there is a better way to dance.

@WojciechMazur
Copy link
Contributor Author

@LeeTibbert let's do this in a way you described

@sjrd sjrd changed the title Fix not reachable definitions of default methods Fix #2012: Fix not reachable definitions of default methods Nov 28, 2020
@sjrd sjrd merged commit 2a90b2a into scala-native:master Nov 28, 2020
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this pull request Nov 28, 2020
This PR builds upon merged PRs scala-native#1937 & scala-native#2040 by annotating the existing
j.u.f.BiFunction.scala and porting its corresponding BiFunctionTest
from Scala.js.

The existing file was edited rather than porting the most current
Scala.js commit because the latter uses lambdas and those
do not play well when using Scala Native with Scala 2.11.
@WojciechMazur WojciechMazur deleted the fix/fix_not_reachable_methods branch February 15, 2021 09:08
ekrich pushed a commit to ekrich/scala-native that referenced this pull request May 21, 2021
…ds (scala-native#2040)

Previously, default method implementations were not used in the
reachability check process.
WojciechMazur added a commit to WojciechMazur/scala-native that referenced this pull request Aug 25, 2021
…ds (scala-native#2040)

Previously, default method implementations were not used in the
reachability check process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimizing in debug mode produces warnings on JUnit test
3 participants