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

No Uses* (again) #2537

Merged
merged 12 commits into from
Sep 23, 2024
Merged

No Uses* (again) #2537

merged 12 commits into from
Sep 23, 2024

Conversation

tokatoka
Copy link
Member

No description provided.

@tokatoka
Copy link
Member Author

Unlike the failed attempt, the objective of this pr is not to reduce the number of trait bounds.
Essentially there are they are two separate problems.

  1. is to prevent UsesState, UsesInput to get propagated to everywhere.
  2. is to remove default methods from traits that actually generate the trait bounds.

In this PR, 1 is the target, and for this I can do it with this independent PR then we can graducally fix 1.

@@ -127,14 +112,16 @@ where
}
}

impl<S, Q, CS, F, M, O> HasQueueCycles for SupportedSchedulers<S, Q, CS, F, M, O>
impl<CS, F, I, M, O, S, Q> HasQueueCycles for SupportedSchedulers<CS, F, I, M, O, S, Q>
Copy link
Member

Choose a reason for hiding this comment

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

I still don't get how more generics is better, here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's better because i get rid of UsesState and UsesInput.
And using UsesState and UsesInput was the wrong solution if you want to reduce the number of generics.

Copy link
Member

Choose a reason for hiding this comment

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

Well having the input associated with a state doesn't sound necessarily wrong

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah so i'm not removing UsesInput from StdState

Copy link
Member Author

@tokatoka tokatoka Sep 20, 2024

Choose a reason for hiding this comment

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

what addison said is that you should only have associated X when you can define the getter to it
for example you can define input() and input_mut() so i can implement UsesInput for StdState

Copy link
Member Author

Choose a reason for hiding this comment

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

but in this PR.
you can't implement fn state() or fn input() for this scheduler, then you don't implement UsesState, UsesInput.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure I fully understand, but just do what you thank and we see how it looks?

@tokatoka
Copy link
Member Author

tokatoka commented Sep 20, 2024

Can you see if this is good to you? @addisoncrump

I was too lazy to remove default implementation from traits (for example, it's totally possible to remove HasCorpus from Scheduler's S if we eliminate th that implementation)
but imo that's another problem and not related to the misuse of Use*

Also this is not trying to fix every occurence of Use*, but make incremental progress towards the end goal while keeping everything working

@tokatoka tokatoka merged commit 93fdbb6 into main Sep 23, 2024
101 checks passed
@tokatoka tokatoka deleted the scheduler_no_use branch September 23, 2024 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants