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

rename Scalar::Bits to Scalar::Raw and bits field to data #61164

Merged
merged 5 commits into from
May 28, 2019
Merged

rename Scalar::Bits to Scalar::Raw and bits field to data #61164

merged 5 commits into from
May 28, 2019

Conversation

RalfJung
Copy link
Member

Also use this opportunity to seal some abstraction leaks (other modules constructing Scalar::Bits directly instead of using a constructor).

r? @oli-obk

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 25, 2019
@oli-obk
Copy link
Contributor

oli-obk commented May 25, 2019

#59276 has priority over this (there will be conflicts) cc @varkor

@oli-obk oli-obk added the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label May 25, 2019
@bors
Copy link
Contributor

bors commented May 25, 2019

☔ The latest upstream changes (presumably #59276) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk
Copy link
Contributor

oli-obk commented May 25, 2019

r=me after rebase

@RalfJung
Copy link
Member Author

I decided to also refactor how we are doing the scalar sanity checks a bit. And I'd like to see how expensive these checks are.

@bors try

@bors
Copy link
Contributor

bors commented May 26, 2019

⌛ Trying commit aad13a1 with merge 2189421...

bors added a commit that referenced this pull request May 26, 2019
@bors
Auto merge of #61164 - RalfJung:scalar, r=<try>
2189421 
rename Scalar::Bits to Scalar::Raw and bits field to data

Also use this opportunity to seal some abstraction leaks (other modules constructing `Scalar::Bits` directly instead of using a constructor).

r? @oli-obk
@RalfJung RalfJung removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label May 26, 2019
@RalfJung
Copy link
Member Author

Oh and I found a bug in the signed pointer offset code that could lead to those sanity checks failing. ;)

@RalfJung RalfJung mentioned this pull request May 26, 2019
Merged
@bors
Copy link
Contributor

bors commented May 26, 2019

☀️ Try build successful - checks-travis
Build commit: 2189421

@RalfJung
Copy link
Member Author

@rust-timer build 2189421

@rust-timer
Copy link
Collaborator

Success: Queued 2189421 with parent 566f3d7, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 2189421: comparison url

@RalfJung
Copy link
Member Author

Timing looks much better than I expected! max <4%. I'll still demote some of these to debug_assert, but that means debug-assert-enabled builds check things much more now than they used to.

@RalfJung
Copy link
Member Author

Now with fewer assertions.

@bors try

@bors
Copy link
Contributor

bors commented May 26, 2019

⌛ Trying commit 829d8de with merge 2eefc6c...

bors added a commit that referenced this pull request May 26, 2019
@bors
Auto merge of #61164 - RalfJung:scalar, r=<try>
2eefc6c 
rename Scalar::Bits to Scalar::Raw and bits field to data

Also use this opportunity to seal some abstraction leaks (other modules constructing `Scalar::Bits` directly instead of using a constructor).

r? @oli-obk
@rust-highfive

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented May 27, 2019

⌛ Trying commit fe19ed8 with merge f3c7924...

bors added a commit that referenced this pull request May 27, 2019
@bors
Auto merge of #61164 - RalfJung:scalar, r=<try>
f3c7924 
rename Scalar::Bits to Scalar::Raw and bits field to data

Also use this opportunity to seal some abstraction leaks (other modules constructing `Scalar::Bits` directly instead of using a constructor).

r? @oli-obk
@bors
Copy link
Contributor

bors commented May 27, 2019

☀️ Try build successful - checks-travis
Build commit: f3c7924

@RalfJung
Copy link
Member Author

@rust-timer build f3c7924

@rust-timer
Copy link
Collaborator

Success: Queued f3c7924 with parent ab7cf71, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit f3c7924: comparison url

@RalfJung
Copy link
Member Author

This looks good perf-wise!

@RalfJung
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented May 27, 2019

📌 Commit fe19ed8 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 27, 2019
@RalfJung
Copy link
Member Author

This is on the critical path to make Miri work again.

@bors p=1

Centril added a commit to Centril/rust that referenced this pull request May 28, 2019
@Centril
Rollup merge of rust-lang#61164 - RalfJung:scalar, r=oli-obk
ede8a6c 
rename Scalar::Bits to Scalar::Raw and bits field to data

Also use this opportunity to seal some abstraction leaks (other modules constructing `Scalar::Bits` directly instead of using a constructor).

r? @oli-obk
@Centril Centril mentioned this pull request May 28, 2019
Closed
Centril added a commit to Centril/rust that referenced this pull request May 28, 2019
@Centril
Rollup merge of rust-lang#61164 - RalfJung:scalar, r=oli-obk
16ef483 
rename Scalar::Bits to Scalar::Raw and bits field to data

Also use this opportunity to seal some abstraction leaks (other modules constructing `Scalar::Bits` directly instead of using a constructor).

r? @oli-obk
@Centril Centril mentioned this pull request May 28, 2019
Merged
bors added a commit that referenced this pull request May 28, 2019
@bors
Auto merge of #61274 - Centril:rollup-23dekk4, r=Centril
7212685 
Rollup of 4 pull requests

Successful merges:

 - #61123 (Allow to specify profiling data output directory as -Zself-profile argument.)
 - #61159 (split core::ptr module into multiple files)
 - #61164 (rename Scalar::Bits to Scalar::Raw and bits field to data)
 - #61250 (Remove special case for *ios* builds in run-make-fulldeps/print-target-list Makefile)

Failed merges:

r? @ghost
matthiaskrgr added a commit to matthiaskrgr/rust-clippy that referenced this pull request May 28, 2019
@matthiaskrgr
rustup rust-lang/rust#61164
c9cc11e 
(which is included in rust-lang/rust#61274)
@bors
Copy link
Contributor

bors commented May 28, 2019

⌛ Testing commit fe19ed8 with merge 7212685...

@bors bors merged commit fe19ed8 into rust-lang:master May 28, 2019
bors added a commit to rust-lang/rust-clippy that referenced this pull request May 28, 2019
@bors
Auto merge of #4152 - matthiaskrgr:rustup_4, r=matthiaskrgr
487e6eb 
rustup rust-lang/rust#61164

(which is included in rust-lang/rust#61274)

changelog: none
@Centril Centril mentioned this pull request May 28, 2019
Closed
bors added a commit to rust-lang/rust-clippy that referenced this pull request May 28, 2019
@bors
Auto merge of #4152 - matthiaskrgr:rustup_4, r=matthiaskrgr
a05f18e 
rustup rust-lang/rust#61164

(which is included in rust-lang/rust#61274)

changelog: none
@RalfJung RalfJung deleted the scalar branch May 29, 2019 08:01
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 30, 2019
@matthiaskrgr
submodules: update clippy from fb33fad to d2f5122
9817f19 
Changes:
````
Rustup to rust-lang#61203
rustup rust-lang#60928
rustup rust-lang#61164 (which is included in rust-lang#61274)
````
bors added a commit that referenced this pull request May 31, 2019
@bors
Auto merge of #61298 - matthiaskrgr:submodule_upd, r=oli-obk
75f4644 
submodules: update clippy from fb33fad to d2f5122

Changes:
````
Rustup to #61203
rustup #60928
rustup #61164 (which is included in #61274)
````

Fixes #61287

r? @oli-obk
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 5, 2020
@matthiaskrgr
submodules: update clippy from fb33fad to d2f5122
4a6d6f4 
Changes:
````
Rustup to rust-lang/rust#61203
rustup rust-lang/rust#60928
rustup rust-lang/rust#61164 (which is included in rust-lang/rust#61274)
````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril left review comments

@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@RalfJung @oli-obk @bors @rust-timer @rust-highfive @Centril