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

Generate BuiltinMethods from simple method and constructor signatures #3444

Merged
merged 9 commits into from
May 12, 2022

Conversation

hubertp
Copy link
Contributor

@hubertp hubertp commented May 10, 2022

Pull Request Description

A low-hanging fruit where we can automate the generation of many
@BuiltinMethod nodes simply from the runtime's methods signatures.
This change introduces another annotation, @Builtin, to distinguish from
@BuiltinType and @BuiltinMethod processing. @Builtin processing will
always be the first stage of processing and its output will be fed to
the latter.

Note that the return type of Array.length() is changed from int to
long because we probably don't want to add a ton of specializations
for the former (see comparator nodes for details) and it is fine to cast
it in a small number of places.

Progress is visible in the number of deleted hardcoded classes.

This is an incremental step towards #181499077.

Important notes

This process does not attempt to cover all cases. Not yet, at least.
We only handle simple methods and constructors (see removed Array boilerplate methods).

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 code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH ./run dist and ./run watch.

A low-hanging fruit where we can automate the generation of many
@BuiltinMethod nodes simply from the runtime's methods signatures.
This change introduces another annotation, @Builtin, to distinguish from
@BuiltinType and @BuiltinMethod processing. @Builtin processing will
always be the first stage of processing and its output will be fed to
the latter.

Note that the return type of Array.length() is changed from `int` to
`long` because we probably don't want to add a ton of specializations
for the former (see comparator nodes for details) and it is fine to cast
it in a small number of places.

Progress is visible in the number of deleted hardcoded classes.

This is an incremental step towards #181499077.
Copy link
Member

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

Nice.

out.println("import " + ownerClazz.fullyQualifiedName() + ";");
out.println();
out.println(
"@BuiltinMethod(type = \""
Copy link
Member

Choose a reason for hiding this comment

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

A processor that generates code for another processor - that's metacyclic!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Overriding supported version in processors manually is the only way to
keep it quiet when using Frgaal. Thanks for the suggestion Jaroslav!
@hubertp hubertp requested a review from kustosz May 11, 2022 10:09
@hubertp hubertp requested a review from 4e6 May 11, 2022 13:21
@hubertp hubertp changed the title Generate simple BuiltinMethods from method signatures Generate BuiltinMethods from simple method signatures May 11, 2022
Extended the initial approach to also generate `Foo.new` builtin methods
when @Builtin is added to the constructor. `NewNode` for `Array` is now
gone. For consistency, the generated one will now be called
`NewArrayNode`.
@hubertp hubertp changed the title Generate BuiltinMethods from simple method signatures Generate BuiltinMethods from simple method and constructor signatures May 11, 2022
@hubertp
Copy link
Contributor Author

hubertp commented May 11, 2022

Now also handling simple constructors = profit!

@hubertp hubertp added the CI: Ready to merge This PR is eligible for automatic merge label May 12, 2022
@mergify mergify bot merged commit a2dae60 into develop May 12, 2022
@mergify mergify bot deleted the wip/hubert/builtin-objects-181499077 branch May 12, 2022 08:42
mergify bot pushed a commit that referenced this pull request May 19, 2022
This is the 2nd part of DSL improvements that allow us to generate a lot of
builtins-related boilerplate code.
- [x] generate multiple method nodes for methods/constructors with varargs
- [x] expanded processing to allow for @Builtin to be added to classes and
and generate @BuiltinType classes
- [x] generate code that wraps exceptions to panic via `wrapException`
annotation element (see @Builtin.WrapException`

Also rewrote @Builtin annotations to be more structured and introduced some nesting, such as
@Builtin.Method or @Builtin.WrapException.

This is part of incremental work and a follow up on #3444.

# Important Notes
Notice the number of boilerplate classes removed to see the impact.
For now only applied to `Array` but should be applicable to other types.
Copy link
Contributor

@kustosz kustosz left a comment

Choose a reason for hiding this comment

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

Awesome! Not sure about the pkg field though, any way for us to get rid of it? And just slap these "somewhere" by convention?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants