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

API: Removed the CallableData trait #265

Merged
merged 2 commits into from
Oct 1, 2023

Conversation

xFrednet
Copy link
Member

The CallableData trait, was designed with the idea to have a general trait for function like things, including FnItem, ClosureExpr, SenFnPrtTy. However, after working with Marker for a bit, I learned that these AST nodes are just too different. At least, too different, to have the CallableData trait as the main API.

I'm also not sure what the use case would be, where a syntactic type should be handled the same way as a FnItem.

This PR simply removes the trait and moves the remaining functions to the SenFnPrtTy type. #194 already removed the trait from FnItem and ClosureExpr.

This PR also deletes the SynClosureTy since it's not actually a syntactic type that can be written ^^

@xFrednet xFrednet added C-enhancement Category: New feature or request A-api Area: Stable API I-api-break Issue: This change will break the public API labels Sep 30, 2023
@xFrednet xFrednet added this to the v0.3.0 milestone Sep 30, 2023
@@ -30,26 +28,6 @@ impl<'ast> SemFnTy<'ast> {
}
}

/// The syntactic representation of a
/// [closure type](https://doc.rust-lang.org/reference/types/closure.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

That's weird how there could be a syntactic representation of a closure type because there is no syntax to denote a type of the closure except for impl Fn*. However, impl Fn* should not be coupled with closures, that's a broader concept.

So I wonder what this struct represented then

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't actually know anymore why I added this struct. My guess is that this is a left over from an old architecture. There was a time, where I planned to merge both syntactic and semantic types into a single representation. My idea was that this would make it easier, to work with types. Then you could compare syntactic and semantic types etc.

However, while working on semantic types, I quickly noticed that these are way more different than I thought. It also became clear that this combination will most likely be painful for everyone involved, with very few benefits.

During the move to split semantic and syntactic types, I just added the Syn prefix to most types. The struct might have been intended to be a semantic type, but got caught in the crossfire.

@xFrednet xFrednet added this pull request to the merge queue Oct 1, 2023
Merged via the queue into rust-marker:master with commit 4fb1ccf Oct 1, 2023
15 checks passed
@xFrednet xFrednet deleted the 000-nuke-callable-data-trait branch October 1, 2023 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-api Area: Stable API C-enhancement Category: New feature or request I-api-break Issue: This change will break the public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants