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

Implement marker traits #6871

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Implement marker traits #6871

wants to merge 8 commits into from

Conversation

ironcev
Copy link
Member

@ironcev ironcev commented Jan 29, 2025

Description

This PR introduces the concept of marker traits to the language. It is the first step towards implementing the ABI errors RFC.

Marker traits are traits automatically generated by the compiler. They represent certain properties of types and cannot be explicitly implemented by developers.

The PR implements a common infrastructure for generating and type-checking marker traits as well as two concrete marker traits:

  • Error: represents a type whose instances can be used as arguments for the panic expression. (The panic expression is yet to be implemented.) In this PR, it is implemented only for string slices. In the upcoming PR it will be implemented for enums annotated with the #[error_type] attribute.
  • Enum: represents an enum type. It is inspired by the Rust's experimental Tuple marker trait and used in this PR as a sample marker trait, to develop and test the marker trait autogeneration.

Marker traits allow developers expression constraints related to certain properties of Sway types. As an illustration, if developers want to express a constraint such is "the error type must be an error enum", they can do it by combining the two marker traits:

fn panic_with_error<E>(err: E) where E: Error + Enum {
    panic err;
}

Note that the generic name Enum is sometimes used in our tests to represent a dummy enum. In tests, it is almost always defined locally, and sometimes explicitly imported, so it will never clash with the Enum marker trait. A single test in which the clash occurred was easily adapted by explicitly importing the dummy Enum.

The PR is the first step in implementing #6765.

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@ironcev ironcev self-assigned this Jan 29, 2025
@ironcev ironcev added compiler General compiler. Should eventually become more specific as the issue is triaged language feature Core language features visible to end users compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Jan 29, 2025
Copy link

codspeed-hq bot commented Jan 29, 2025

CodSpeed Performance Report

Merging #6871 will not alter performance

Comparing ironcev/marker-traits (c02f83e) with master (55358da)

Summary

✅ 22 untouched benchmarks

@ironcev ironcev marked this pull request as ready for review January 29, 2025 23:51
@ironcev ironcev requested review from a team as code owners January 29, 2025 23:51
@IGI-111
Copy link
Contributor

IGI-111 commented Jan 30, 2025

I'm not sure we want to restrict ABI errors to be necessarily Enums, what's the rationale behind having to check this particular constraints in a trait impl instead of being more generic?

@ironcev ironcev enabled auto-merge (squash) January 30, 2025 09:59
@ironcev
Copy link
Member Author

ironcev commented Jan 30, 2025

I'm not sure we want to restrict ABI errors to be necessarily Enums, what's the rationale behind having to check this particular constraints in a trait impl instead of being more generic?

@IGI-111 The ABI errors are not necessarily Enums, on the contrary. Currently, in this PR, they are implemented only for string slices. Perhaps is the example given in the description misleading. The idea of the example was to demonstrate the usage of marker traits in general - writing generic code that has constraints on different type properties.

Error will be implemented for #[error_type] enums in the upcoming PR. If we decide later to have other types supporting Error, we can always extend the support. E.g., supporting u64 and still alow developers to use error codes.

But the question leads me to the questionable purpose of the Enum trait, so a bit of a background for having it in this PR.

Since the full-blown Error on enums is coming in the next PR, to build and test the general infrastructure for marker traits, I needed a suitable, simple marker trait with auto-implementations on elgible types. Enum marker was a simple choice inspired by the Rust's experimental Tuple marker trait.

Once started playing with it, I realized that it actually fits nicely with the Error marker in case developers intentionally want to restrict errors to be enums. E.g., using string slices in panic increases code size to embed slices. A design agreement to stick to error enums, where the error strings are not embedded in code, could be then enforced on the API that panics with given error in a way shown in the description.

This might be a far stretched use case for a real world application, but as with all new language features, it is hard to anticipate how far devs will take it. I am fine with removing Enum marker trait in the upcoming PR if we agree so. But by then, in this PR, to implement and tests the overall approach to marker traits, we need a simple sample trait to start with.

I'll adapt the PR description to better convey the choices done in this PR and to avoid misleading example.

Copy link
Contributor

@jjcnn jjcnn left a comment

Choose a reason for hiding this comment

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

All looks good except CallPath::to_string_with_args, which I think needs to be tweaked.

sway-core/src/language/call_path.rs Outdated Show resolved Hide resolved
@ironcev ironcev requested a review from jjcnn January 31, 2025 13:49
@ironcev ironcev changed the title Marker traits Implement marker traits Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler General compiler. Should eventually become more specific as the issue is triaged language feature Core language features visible to end users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants