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

Fluent Java Interop & Method Dispatch Refactor #1443

Merged
merged 14 commits into from
Feb 1, 2021
Merged

Conversation

kustosz
Copy link
Contributor

@kustosz kustosz commented Feb 1, 2021

Pull Request Description

Removes currying for polyglot calls while also removing the need for vectored arguments and unifying field/method access patterns. Also pushes the method dispatch logic to a truffle library, so that our resolution logic can be more object-oriented and less "let me try to handle every single type of value in one node".

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the Scala, Java, and Rust style guides.
  • All documentation and configuration conforms to the markdown and YAML style guides.
  • All code has been tested where possible.

@kustosz kustosz added Type: Enhancement --breaking Important: a change that will break a public API or user-facing behaviour p-highest Should be completed ASAP labels Feb 1, 2021
@kustosz kustosz self-assigned this Feb 1, 2021
fst = iterator.first []
nxt = iterator.next []
fst = iterator.first
nxt = iterator.next
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely we can use next here now, given that we no longer have UFCS! Other locations too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We sure can. But this PR had too much boy-scouting in other places already, it's just too much for one person :P

@@ -4,5 +4,5 @@ import Builtins
## Checks whether an object is an instance of a given class.
Builtins.Java.is_instance : Any -> Any -> Boolean
Builtins.Java.is_instance object class =
class_object = Polyglot.get_member class "class"
class_object.isInstance [object]
class_object = class.class
Copy link
Contributor

Choose a reason for hiding this comment

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

:'D

argumentsExecutionMode,
isTail);
} catch (MethodDispatchLibrary.NoSuchMethodException e) {
CompilerDirectives.transferToInterpreter();
Copy link
Contributor

Choose a reason for hiding this comment

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

We really need a consistent rule for whether we use this or not when throwing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair. Let's stick with don't use for new code.

Comment on lines 21 to +23
build_map size =
rand = Random.new [].to_array
0.up_to size . fold Map.empty (m -> i -> m.insert (rand.nextInt [10000]) i)
rand = Random.new
0.up_to size . fold Map.empty (m -> i -> m.insert (rand.nextInt 10000) i)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you fix the other benchmarks? They use polyglot a lot.

@kustosz kustosz merged commit f277517 into main Feb 1, 2021
@kustosz kustosz deleted the wip/mk/poly-common branch February 1, 2021 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--breaking Important: a change that will break a public API or user-facing behaviour p-highest Should be completed ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants