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.

Sign up for GitHub

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

Jump to bottom

Remove Trait constraint for Port::new() #188

Merged
merged 1 commit into from
Sep 30, 2020
Merged

Remove Trait constraint for Port::new() #188

merged 1 commit into from
Sep 30, 2020

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented Sep 29, 2020 •
edited
Loading

Right now we use Traits with const fn in two different ways:

  1. PortRead/PortWrite/PortReadWrite, which can never have const fn implementaitons
  2. PageSize which always has a const fn implementation (as it's just two associated consts)

The use of (1) can be eliminated by only constraining the Port::read method instead of the Port::new method. This is similar to what the standard library does for Once::new.

Signed-off-by: Joe Richey [email protected]

@josephlr josephlr requested a review from phil-opp September 29, 2020 11:04
phil-opp
phil-opp approved these changes Sep 29, 2020
View reviewed changes
Copy link
Member

@phil-opp phil-opp left a comment

Choose a reason for hiding this comment

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

LGTM!

@josephlr josephlr merged commit 7fcea5d into rust-osdev:master Sep 30, 2020
@josephlr josephlr deleted the constfn branch September 30, 2020 09:26
phil-opp added a commit that referenced this pull request Oct 31, 2020
@phil-opp
Update changelog for #165, #191, #184, and #188
d8237f3
phil-opp added a commit that referenced this pull request Nov 11, 2020
@phil-opp
Update changelog for #165, #191, #184, and #188
579bac8
@phil-opp phil-opp mentioned this pull request Dec 28, 2020
Merged
@dependabot dependabot bot mentioned this pull request Mar 17, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phil-opp phil-opp phil-opp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@josephlr @phil-opp