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

(old PR) WIP: Add yoke crate #696

Closed
wants to merge 1 commit into from
Closed

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented May 1, 2021

Superseded by #705

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/yokeable.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/cart.rs is different
  • utils/yoke/src/yokeable.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/yoke.rs is different
  • utils/yoke/src/yokeable.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/yokeable.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

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

Nifty!

}

unsafe fn make(from: Cow<'a, T>) -> Self {
debug_assert!(mem::size_of::<Cow<'a, T>>() == mem::size_of::<Self>());
Copy link
Member

Choose a reason for hiding this comment

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

static_assert instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

rust doesn't have that; because Rust generics are not typechecked at substitution (unlike templates)

Copy link
Member

Choose a reason for hiding this comment

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

So, in other words, this is impossible to test at build time, even if you were to use https://docs.rs/static_assertions/1.1.0/static_assertions/ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and the static assertions crate uses tricks like mem::transmute which doesn't work here.

It might be possible to test using consts. I'm not sure, but it would be super happy

utils/yoke/src/yokeable.rs Outdated Show resolved Hide resolved
utils/yoke/src/yokeable.rs Outdated Show resolved Hide resolved
cow: Cow<'static, str>,
}

// The following code should NOT compile!!!
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe! I'm not a huge fan of pulling in additional tooling for this

Copy link
Member Author

Choose a reason for hiding this comment

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

Rustdoc has compile_fail, used it

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/cart.rs is no longer changed in the branch
  • utils/yoke/src/lib.rs is different
  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • Cargo.lock is different
  • utils/yoke/Cargo.toml is different
  • utils/yoke/src/lib.rs is different
  • utils/yoke/src/yoke.rs is different
  • utils/yoke/src/yokeable.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/lib.rs is different
  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/cart.rs is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/lib.rs is different
  • utils/yoke/src/yoke.rs is different
  • utils/yoke/src/yokeable.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@codecov-commenter
Copy link

Codecov Report

Merging #696 (b34fada) into main (478c4a9) will decrease coverage by 0.44%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #696      +/-   ##
==========================================
- Coverage   74.18%   73.74%   -0.45%     
==========================================
  Files         171      177       +6     
  Lines       10802    10869      +67     
==========================================
+ Hits         8014     8015       +1     
- Misses       2788     2854      +66     
Impacted Files Coverage Δ
utils/yoke/src/cart.rs 0.00% <0.00%> (ø)
utils/yoke/src/lib.rs 100.00% <100.00%> (ø)
components/capi/src/pluralrules.rs 0.00% <0.00%> (ø)
components/capi/src/fixed_decimal.rs 0.00% <0.00%> (ø)
components/capi/src/macros.rs 0.00% <0.00%> (ø)
components/capi/src/decimal.rs 0.00% <0.00%> (ø)
components/capi/src/custom_writeable.rs 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 478c4a9...b34fada. Read the comment docs.

@coveralls
Copy link

coveralls commented May 5, 2021

Pull Request Test Coverage Report for Build 6208b9c6cbf3234e62de2b3be7fc7d708bd4b61b-PR-696

  • 1 of 4 (25.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.02%) to 74.097%

Changes Missing Coverage Covered Lines Changed/Added Lines %
utils/yoke/src/cart.rs 0 3 0.0%
Totals Coverage Status
Change from base Build c6582ccddf87d49a44e2d4e54604543365c76bff: -0.02%
Covered Lines: 8127
Relevant Lines: 10968

💛 - Coveralls

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/cart.rs is different
  • utils/yoke/src/yokeable.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • Cargo.lock is different
  • utils/yoke/Cargo.toml is different
  • utils/yoke/src/cart.rs is different
  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@Manishearth
Copy link
Member Author

Current status: hitting rust-lang/rust#84937 with using attach_to_cart(), temporarily worked around it by introducing attach_to_cart_but_worse() so that I can write tests and examples that work.

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/src/yoke.rs is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • utils/yoke/Cargo.toml is different
  • utils/yoke/src/cart.rs is different
  • utils/yoke/src/lib.rs is different
  • utils/yoke/src/yoke.rs is different
  • utils/yoke/src/yokeable.rs is different
  • utils/yoke/src/zerovec_impls.rs is now changed in the branch
  • utils/yoke/tests/bincode.rs is now changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@Manishearth
Copy link
Member Author

Closing because the jira bot has left too many comments, going to open a new PR for review

@Manishearth Manishearth closed this May 5, 2021
@Manishearth Manishearth mentioned this pull request May 5, 2021
@Manishearth
Copy link
Member Author

#705

@Manishearth Manishearth changed the title WIP: Add yoke crate (old PR) WIP: Add yoke crate May 5, 2021
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.

4 participants