Skip to content

Introducing default trait methods#128

Merged
Champii merged 11 commits intodevelopfrom
trait_default_method
Jun 23, 2022
Merged

Introducing default trait methods#128
Champii merged 11 commits intodevelopfrom
trait_default_method

Conversation

@Champii
Copy link
Copy Markdown
Owner

@Champii Champii commented Jun 20, 2022

No description provided.

@Champii Champii added the enhancement New feature or request label Jun 20, 2022
@Champii Champii self-assigned this Jun 20, 2022
@Champii
Copy link
Copy Markdown
Owner Author

Champii commented Jun 20, 2022

This PR introduces default methods for traits:

trait Foo a
  @default_method: -> @

impl Foo Int64

main: -> (2).default_method!

There is still a bug that needs a full empty line after the impl
@Champii
Copy link
Copy Markdown
Owner Author

Champii commented Jun 20, 2022

We need a mecanism to allow impl to override a default method.

Copy link
Copy Markdown
Owner Author

@Champii Champii left a comment

Choose a reason for hiding this comment

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

Should add relevant issues from comments

Comment thread src/lib/parser/default_impl_populator.rs
Comment thread src/lib/parser/default_impl_populator.rs Outdated
Comment thread src/lib/parser/mod.rs
@Champii Champii requested a review from oraqlle June 20, 2022 19:02
@Champii Champii marked this pull request as ready for review June 20, 2022 19:02
@Champii Champii force-pushed the trait_default_method branch from e0fe5db to 3ac5028 Compare June 20, 2022 19:16
Copy link
Copy Markdown
Owner Author

@Champii Champii left a comment

Choose a reason for hiding this comment

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

Need tests

@Champii
Copy link
Copy Markdown
Owner Author

Champii commented Jun 21, 2022

Need doc

@oraqlle
Copy link
Copy Markdown
Collaborator

oraqlle commented Jun 21, 2022

What do you need me to review. The code or the idea. Also, what does a default method mean. As in that in the trait definition, we can declare a default implementation of a trait method that might be overridden?

@Champii
Copy link
Copy Markdown
Owner Author

Champii commented Jun 21, 2022

What do you need me to review. The code or the idea.

Principally the idea, but I would not refuse you picking a quick glance at the code, you might have some useful insights I'm missing

Also, what does a default method mean. As in that in the trait definition, we can declare a default implementation of a trait method that might be overridden?

Yes, that's it. That way you can offer some basic logic that you can plug effortless into any type.
That, in coordination with a future "trait dependency" (like a trait bound in Rust), we could have a powerful abstract system.

@Champii Champii linked an issue Jun 22, 2022 that may be closed by this pull request
@Champii
Copy link
Copy Markdown
Owner Author

Champii commented Jun 22, 2022

I think we are all good for this one, waiting for your review before merging :)

@oraqlle
Copy link
Copy Markdown
Collaborator

oraqlle commented Jun 23, 2022

What is another POD type in Rock besides Int64?

Copy link
Copy Markdown
Collaborator

@oraqlle oraqlle left a comment

Choose a reason for hiding this comment

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

Looks Good!

Tested the old README.md stuff, had to get some of the updated versions such as the new impl but other than that it works.

Utilising the default methods is super clean and efficient. One thing I will point out for a future update is that when using the @ self symbol, the trait default trait declaration allows for a dot to preceed the method name as well as not allowing for it. ie. It doesn't enforce a dot or not a dot.
eg.

trait Foo a
    @de: -> @print!

This is the same as

trait Foo a
    @de: -> @.print!

We might want to enforce a particular syntax for consistency. Personally, I like the no-dot as it implies a sort-of auto dot.

@Champii
Copy link
Copy Markdown
Owner Author

Champii commented Jun 23, 2022

What is another POD type in Rock besides Int64?

You can find all the currently implemented primitive types of Rock at https://github.com/Champii/Rock/blob/master/src/lib/ty/primitive_type.rs#L4
Note that struct is part of the greater Type but is still implemented as C struct type by LLVM

Tested the old README.md stuff, had to get some of the updated versions such as the new impl but other than that it works.

Is there some stuff I forgot to update in the readme ?
Also, so that you know, most of the readme stuff is replicated in the unit tests, that are run at each commit. There is still the matter of a mistake, so maybe we could have a system to use the actual code of the working tests to populate the readme. That way we are sure everything works by default, and you won't have to manually try each snippet yourself :)

One thing I will point out for a future update is that when using the @ self symbol, the trait default trait declaration allows for a dot to preceed the method name as well as not allowing for it. ie. It doesn't enforce a dot or not a dot.

This stuff is actually intended, and it will be difficult to enforce it. The primary concern is about the future macro system, that will have to make generic syntactic constructs that work for every identifier type. As we could write stuff like

macro my_macro
  $my_ident:ident =>
    $my_ident.print!

When applied to @, it expands to @.print! and this should still be valid. I'm also a partisan of the point-free notation for @, but I guess it will be easier to enforce it as a convention or best-practice.

@Champii Champii merged commit 7efb512 into develop Jun 23, 2022
@Champii Champii deleted the trait_default_method branch June 23, 2022 13:19
Champii added a commit that referenced this pull request Jun 26, 2022
* Bump version

* Update README to show the right version of llvm

* Add the self returning method with `@->`

* Add tests for self returning function

* Add an embryo of documentation with mdbook

* Documentation

* Fix doc about struct

* Doc

* Add \0 as escaped char and generalize escaped char parsing (#125)

* Add \0 as escaped char and generalize escaped char parsing

* Inline parser variable in parse_char method

* Readme

* Readme

* Escaped chars (#132)

* Add \0 as escaped char and generalize escaped char parsing

* Inline parser variable in parse_char method

* Better unescaped handling

* Fix escaped backslack and add tests

* Restored string unescape

* Fixed the `\0` escaped char

* Doc

* Introducing default trait methods (#128)

* Introducing default trait methods

* Allow for empty impl

* Readme

* Add tests

* Allow for default method overriding

* Add tests for default method overriding

* Doc

* Fix dot notation newline (#140)

* Fix dot notation

by restricting newline in dot notation when parsing an argument

* Readme

* Struct fields are no more reordered. (#138)

* Struct fields are no more reordered.

* Removed comment

* Removed some old readme entry

* Submodule parser errors (#137)

* Subparser now fails when error

* Transformed the module declaration error to failure

* Fixed the diagnostic in submodule not showing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow for default method override in impl [ENHANCE] Print and Println distinction Trait default method implementation

2 participants