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

Add `Default` to `std::alloc::System` #59451

Merged
merged 1 commit into from Mar 28, 2019

Conversation

Projects
None yet
6 participants
@TimDiekmann
Copy link
Contributor

TimDiekmann commented Mar 26, 2019

System is a unit struct, thus, it can be constructed without any additional information. Therefore Default is a noop. However, in generic code, a T: Default may happen as in

#[derive(Default)]
struct Foo<A> {
    allocator: A
}

Does this need a feature gate?
Should I also add PartialEq/Eq/PartialOrd/Ord/Hash?

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Mar 26, 2019

r? @rkruppe

(rust_highfive has picked a reviewer for you, use r? to override)

@Centril Centril added the relnotes label Mar 27, 2019

@Centril Centril added this to the 1.35 milestone Mar 27, 2019

@Centril

This comment has been minimized.

Copy link
Contributor

Centril commented Mar 27, 2019

Randomly reassigning to T-libs member, r? @sfackler

@rust-highfive rust-highfive assigned sfackler and unassigned rkruppe Mar 27, 2019

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Mar 27, 2019

Trait impls are automatically stable, so no need for a feature gate here.

Let's not add Hash/Eq/etc yet - they're a bit silly since this type's a unit struct after all.

@bors r+ rollup

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 27, 2019

📌 Commit 8733b2a has been approved by sfackler

@TimDiekmann

This comment has been minimized.

Copy link
Contributor Author

TimDiekmann commented Mar 27, 2019

Let's not add Hash/Eq/etc yet - they're a bit silly since this type's a unit struct after all.

I think this was the exact reason, why Default wasn't implemented. IMO a trait should be implemented regardless of its structure as generic code does not care if it's a unit struct or not.

Centril added a commit to Centril/rust that referenced this pull request Mar 27, 2019

Rollup merge of rust-lang#59451 - TimDiekmann:patch-1, r=sfackler
Add `Default` to `std::alloc::System`

`System` is a unit struct, thus, it can be constructed without any additional information. Therefore `Default` is a noop. However, in generic code, a `T: Default` may happen as in

```rust
#[derive(Default)]
struct Foo<A> {
    allocator: A
}
```

Does this need a feature gate?
Should I also add `PartialEq/Eq/PartialOrd/Ord/Hash`?

bors added a commit that referenced this pull request Mar 27, 2019

Auto merge of #59457 - Centril:rollup, r=Centril
Rollup of 13 pull requests

Successful merges:

 - #57293 (Make some lints incremental)
 - #57565 (syntax: Remove warning for unnecessary path disambiguators)
 - #58253 (librustc_driver => 2018)
 - #58581 (Refactor generic parameter encoder functions)
 - #58717 (Add FromStr impl for NonZero types)
 - #59283 (Make ASCII case conversions more than 4× faster)
 - #59284 (adjust MaybeUninit API to discussions)
 - #59372 (add rustfix-able suggestions to trim_{left,right} deprecations)
 - #59421 (Reject integer suffix when tuple indexing)
 - #59429 (When moving out of a for loop head, suggest borrowing it in nll mode)
 - #59430 (Renames `EvalContext` to `InterpretCx`)
 - #59436 (Update jemalloc-sys to version 0.3.0)
 - #59451 (Add `Default` to `std::alloc::System`)

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Mar 27, 2019

Rollup merge of rust-lang#59451 - TimDiekmann:patch-1, r=sfackler
Add `Default` to `std::alloc::System`

`System` is a unit struct, thus, it can be constructed without any additional information. Therefore `Default` is a noop. However, in generic code, a `T: Default` may happen as in

```rust
#[derive(Default)]
struct Foo<A> {
    allocator: A
}
```

Does this need a feature gate?
Should I also add `PartialEq/Eq/PartialOrd/Ord/Hash`?

Centril added a commit to Centril/rust that referenced this pull request Mar 27, 2019

Rollup merge of rust-lang#59451 - TimDiekmann:patch-1, r=sfackler
Add `Default` to `std::alloc::System`

`System` is a unit struct, thus, it can be constructed without any additional information. Therefore `Default` is a noop. However, in generic code, a `T: Default` may happen as in

```rust
#[derive(Default)]
struct Foo<A> {
    allocator: A
}
```

Does this need a feature gate?
Should I also add `PartialEq/Eq/PartialOrd/Ord/Hash`?

bors added a commit that referenced this pull request Mar 27, 2019

Auto merge of #59466 - Centril:rollup, r=Centril
Rollup of 17 pull requests

Successful merges:

 - #57293 (Make some lints incremental)
 - #57565 (syntax: Remove warning for unnecessary path disambiguators)
 - #58253 (librustc_driver => 2018)
 - #58717 (Add FromStr impl for NonZero types)
 - #58837 (librustc_interface => 2018)
 - #59268 (Add suggestion to use `&*var` when `&str: From<String>` is expected)
 - #59283 (Make ASCII case conversions more than 4× faster)
 - #59284 (adjust MaybeUninit API to discussions)
 - #59372 (add rustfix-able suggestions to trim_{left,right} deprecations)
 - #59393 (Refactor tuple comparison tests)
 - #59421 (Reject integer suffix when tuple indexing)
 - #59430 (Renames `EvalContext` to `InterpretCx`)
 - #59439 (Generalize diagnostic for `x = y` where `bool` is the expected type)
 - #59449 (fix: Make incremental artifact deletion more robust)
 - #59451 (Add `Default` to `std::alloc::System`)
 - #59459 (Add some tests)
 - #59460 (Include id in Thread's Debug implementation)

Failed merges:

r? @ghost

cuviper added a commit to cuviper/rust that referenced this pull request Mar 28, 2019

Rollup merge of rust-lang#59451 - TimDiekmann:patch-1, r=sfackler
Add `Default` to `std::alloc::System`

`System` is a unit struct, thus, it can be constructed without any additional information. Therefore `Default` is a noop. However, in generic code, a `T: Default` may happen as in

```rust
#[derive(Default)]
struct Foo<A> {
    allocator: A
}
```

Does this need a feature gate?
Should I also add `PartialEq/Eq/PartialOrd/Ord/Hash`?

bors added a commit that referenced this pull request Mar 28, 2019

Auto merge of #59471 - cuviper:rollup, r=cuviper
Rollup of 18 pull requests

Successful merges:

 - #57293 (Make some lints incremental)
 - #57565 (syntax: Remove warning for unnecessary path disambiguators)
 - #58253 (librustc_driver => 2018)
 - #58837 (librustc_interface => 2018)
 - #59268 (Add suggestion to use `&*var` when `&str: From<String>` is expected)
 - #59283 (Make ASCII case conversions more than 4× faster)
 - #59284 (adjust MaybeUninit API to discussions)
 - #59372 (add rustfix-able suggestions to trim_{left,right} deprecations)
 - #59390 (Make `ptr::eq` documentation mention fat-pointer behavior)
 - #59393 (Refactor tuple comparison tests)
 - #59420 ([CI] record docker image info for reuse)
 - #59421 (Reject integer suffix when tuple indexing)
 - #59430 (Renames `EvalContext` to `InterpretCx`)
 - #59439 (Generalize diagnostic for `x = y` where `bool` is the expected type)
 - #59449 (fix: Make incremental artifact deletion more robust)
 - #59451 (Add `Default` to `std::alloc::System`)
 - #59459 (Add some tests)
 - #59460 (Include id in Thread's Debug implementation)

Failed merges:

r? @ghost

@bors bors merged commit 8733b2a into rust-lang:master Mar 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.