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

Make Uniform and its helper traits use arguments of type Borrow<X> #506

Merged
merged 1 commit into from
Jun 13, 2018
Merged

Make Uniform and its helper traits use arguments of type Borrow<X> #506

merged 1 commit into from
Jun 13, 2018

Conversation

sicking
Copy link
Contributor

@sicking sicking commented Jun 12, 2018

Fixes #501.

@sicking
Copy link
Contributor Author

sicking commented Jun 12, 2018

Oh, and I should add that this doesn't change the syntax of anyone using Uniform or gen_range, other than possibly making life more complicated for the compiler to deduce types in some cases.

But it'll definitely break anyone implementing UniformSampler

@dhardy dhardy added the D-review Do: needs review label Jun 12, 2018
pitdicker
pitdicker approved these changes Jun 12, 2018
View reviewed changes
Copy link
Contributor

@pitdicker pitdicker left a comment

Choose a reason for hiding this comment

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

Can you document a little bit about the rationale for Borrow in the section "Extending Uniform to support a custom type"?

src/lib.rs Outdated
@@ -935,16 +937,15 @@ mod test {
fn test_gen_range() {
let mut r = rng(101);
for _ in 0..1000 {
let a = r.gen_range(-3, 42);
let a = r.gen_range(-3i8, 42);
Copy link
Contributor

Choose a reason for hiding this comment

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

The extra i8 is just to make the test more interesting, not because troubles with type inference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's there just to make the test more interesting. I can add one without explicit type too.

@sicking sicking force-pushed the borrow branch 2 times, most recently from 8bec2de to 2615d1c Compare June 12, 2018 19:58
@dhardy dhardy merged commit 6c3ac5b into rust-random:master Jun 13, 2018
@sicking sicking mentioned this pull request Jun 15, 2018
Merged
@dhardy dhardy mentioned this pull request Jun 20, 2018
Closed
28 tasks
@sicking sicking deleted the borrow branch June 21, 2018 08:23
@sync-by-unito sync-by-unito bot mentioned this pull request Dec 15, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pitdicker pitdicker pitdicker approved these changes

Assignees
No one assigned
Labels
D-review Do: needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sicking @pitdicker @dhardy