Skip to content

Correct internal BitvSet 0-padding, fixes #16542 #16559

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

Merged
merged 1 commit into from
Aug 18, 2014
Merged

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Aug 17, 2014

These were the only differing-size-based errors I noticed. Might be more.

@apoelstra
Copy link
Contributor

Clever fix! I like the use of chain.

@Gankra
Copy link
Contributor Author

Gankra commented Aug 17, 2014

:embarassed:

Fixing Eq doesn't change the behaviour of cmp, which is still inherited. Will patch asap.

@Gankra
Copy link
Contributor Author

Gankra commented Aug 17, 2014

Sorry about that :stillembarassed:

This is a simpler, more robust solution.

@apoelstra
Copy link
Contributor

One thing I notice here is that PartialOrd should define an ordering defined by set inclusion. (This is my intuition based on the type having Set in the name anyway.) This isn't a total ordering, so you should drop the implementation of Ord.

As mentioned on IRC, Ord is apparently also implemented for TreeMap with the same crazy semantics (lexicographical ordering of elements in sorted order, we think). So maybe this should be done in a separate change with discussion on Discourse.

b.mask_words(0).chain(iter::Repeat::new(0u).enumerate().take(0).skip(0)))
} else {
(a.mask_words(0).chain(iter::Repeat::new(0u).enumerate().take(0).skip(0)),
b.mask_words(0).chain(iter::Repeat::new(0u).enumerate().take(a_len).skip(b_len)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can .take(n).skip(m) be replaced with .take(n - m)? It'd be a little clearer what's going on.

Edit: Never mind, I missed the .enumerate().

@apoelstra
Copy link
Contributor

Regarding the actual code, it looks correct to me.

bors added a commit that referenced this pull request Aug 18, 2014
These were the only differing-size-based errors I noticed. Might be more.
@bors bors closed this Aug 18, 2014
@bors bors merged commit 8c9bdda into rust-lang:master Aug 18, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 18, 2024
…nicola

Set documentation field in SCIP from doc comment

Previously, the documentation field was the same as the text shown to users when they hover over that symbol. The documentation should really just be the doc comment, and as of rust-lang#16179 the signature is already stored in the signatureDocumentation field.
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