`rustc_span::symbol` defines some things that are re-exported from
`rustc_span`, such as `Symbol` and `sym`. But it doesn't re-export some
closely related things such as `Ident` and `kw`. So you can do `use
rustc_span::{Symbol, sym}` but you have to do `use
rustc_span::symbol::{Ident, kw}`, which is inconsistent for no good
reason.
This commit re-exports `Ident`, `kw`, and `MacroRulesNormalizedIdent`,
and changes many `rustc_span::symbol::` qualifiers in `compiler/` to
`rustc_span::`. This is a 200+ net line of code reduction, mostly
because many files with two `use rustc_span` items can be reduced to
one.
Do not add leading asterisk in the `PartialEq`
I think we should address this issue, however I am not exactly sure, if this is the right way to do it. It is related to the #123056.
Imagine the simplified code:
```rust
trait MyTrait {}
impl PartialEq for dyn MyTrait {
fn eq(&self, _other: &Self) -> bool {
true
}
}
#[derive(PartialEq)]
enum Bar {
Foo(Box<dyn MyTrait>),
}
```
On the nightly compiler, the `derive` produces invalid code with the weird error message:
```
error[E0507]: cannot move out of `*__arg1_0` which is behind a shared reference
--> src/main.rs:11:9
|
9 | #[derive(PartialEq)]
| --------- in this derive macro expansion
10 | enum Things {
11 | Foo(Box<dyn MyTrait>),
| ^^^^^^^^^^^^^^^^ move occurs because `*__arg1_0` has type `Box<dyn MyTrait>`, which does not implement the `Copy` trait
|
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)
```
It may be related to the perfect derive problem, although requiring the _type_ to be `Copy` seems unfortunate because it is not necessary. Besides, we are adding the extra dereference only for the diagnostics?
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\clone.rs:160:9
|
160 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\cmp\partial_ord.rs:72:9
|
72 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\cmp\partial_eq.rs:19:18
|
19 | fn cs_eq(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\cmp\ord.rs:42:19
|
42 | pub fn cs_cmp(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_>) -> BlockOrExpr {
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:917:13
|
917 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:1406:13
|
1406 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:1157:13
|
1157 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:1103:13
|
1103 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:1080:13
|
1080 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:859:13
|
859 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:805:13
|
805 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:467:13
|
467 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
warning: this argument is a mutable reference, but not used mutably
--> compiler\rustc_builtin_macros\src\deriving\generic\mod.rs:457:13
|
457 | cx: &mut ExtCtxt<'_>,
| ^^^^^^^^^^^^^^^^ help: consider changing to: `&ExtCtxt<'_>`
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_ref_mut
If you do `derive(PartialEq)` on a packed struct, the output shown by
`-Zunpretty=expanded` includes expressions like this:
```
{ self.x } == { other.x }
```
This is invalid syntax. This doesn't break compilation, because the AST
nodes are constructed within the compiler. But it does mean anyone using
`-Zunpretty=expanded` output as a guide for hand-written impls could get
a nasty surprise.
This commit fixes things by instead using this form:
```
({ self.x }) == ({ other.x })
```
Currently, deriving on packed structs has some non-trivial limitations,
related to the fact that taking references on unaligned fields is UB.
The current approach to field accesses in derived code:
- Normal case: `&self.0`
- In a packed struct that derives `Copy`: `&{self.0}`
- In a packed struct that doesn't derive `Copy`: `&self.0`
Plus, we disallow deriving any builtin traits other than `Default` for any
packed generic type, because it's possible that there might be
misaligned fields. This is a fairly broad restriction.
Plus, we disallow deriving any builtin traits other than `Default` for most
packed types that don't derive `Copy`. (The exceptions are those where the
alignments inherently satisfy the packing, e.g. in a type with
`repr(packed(N))` where all the fields have alignments of `N` or less
anyway. Such types are pretty strange, because the `packed` attribute is
not having any effect.)
This commit introduces a new, simpler approach to field accesses:
- Normal case: `&self.0`
- In a packed struct: `&{self.0}`
In the latter case, this requires that all fields impl `Copy`, which is
a new restriction. This means that the following example compiles under
the old approach and doesn't compile under the new approach.
```
#[derive(Debug)]
struct NonCopy(u8);
#[derive(Debug)
#[repr(packed)]
struct MyType(NonCopy);
```
(Note that the old approach's support for cases like this was brittle.
Changing the `u8` to a `u16` would be enough to stop it working. So not
much capability is lost here.)
However, the other constraints from the old rules are removed. We can now
derive builtin traits for packed generic structs like this:
```
trait Trait { type A; }
#[derive(Hash)]
#[repr(packed)]
pub struct Foo<T: Trait>(T, T::A);
```
To allow this, we add a `T: Copy` bound in the derived impl and a `T::A:
Copy` bound in where clauses. So `T` and `T::A` must impl `Copy`.
We can now also derive builtin traits for packed structs that don't derive
`Copy`, so long as the fields impl `Copy`:
```
#[derive(Hash)]
#[repr(packed)]
pub struct Foo(u32);
```
This includes types that hand-impl `Copy` rather than deriving it, such as the
following, that show up in winapi-0.2:
```
#[derive(Clone)]
#[repr(packed)]
struct MyType(i32);
impl Copy for MyType {}
```
The new approach is simpler to understand and implement, and it avoids
the need for the `unsafe_derive_on_repr_packed` check.
One exception is required for backwards-compatibility: we allow `[u8]`
fields for now. There is a new lint for this,
`byte_slice_in_packed_struct_with_derive`.
There is code for converting `Attribute` (syntactic) to `MetaItem`
(semantic). There is also code for the reverse direction. The reverse
direction isn't really necessary; it's currently only used when
generating attributes, e.g. in `derive` code.
This commit adds some new functions for creating `Attributes`s directly,
without involving `MetaItem`s: `mk_attr_word`, `mk_attr_name_value_str`,
`mk_attr_nested_word`, and
`ExtCtxt::attr_{word,name_value_str,nested_word}`.
These new methods replace the old functions for creating `Attribute`s:
`mk_attr_inner`, `mk_attr_outer`, and `ExtCtxt::attribute`. Those
functions took `MetaItem`s as input, and relied on many other functions
that created `MetaItems`, which are also removed: `mk_name_value_item`,
`mk_list_item`, `mk_word_item`, `mk_nested_word_item`,
`{MetaItem,MetaItemKind,NestedMetaItem}::token_trees`,
`MetaItemKind::attr_args`, `MetaItemLit::{from_lit_kind,to_token}`,
`ExtCtxt::meta_word`.
Overall this cuts more than 100 lines of code and makes thing simpler.
Add the `#[derive_const]` attribute
Closes#102371. This is a minimal patchset for the attribute to work. There are no restrictions on what traits this attribute applies to.
r? `````@oli-obk`````
`rustc_data_structures::thin_vec::ThinVec` looks like this:
```
pub struct ThinVec<T>(Option<Box<Vec<T>>>);
```
It's just a zero word if the vector is empty, but requires two
allocations if it is non-empty. So it's only usable in cases where the
vector is empty most of the time.
This commit removes it in favour of `thin_vec::ThinVec`, which is also
word-sized, but stores the length and capacity in the same allocation as
the elements. It's good in a wider variety of situation, e.g. in enum
variants where the vector is usually/always non-empty.
The commit also:
- Sorts some `Cargo.toml` dependency lists, to make additions easier.
- Sorts some `use` item lists, to make additions easier.
- Changes `clean_trait_ref_with_bindings` to take a
`ThinVec<TypeBinding>` rather than a `&[TypeBinding]`, because this
avoid some unnecessary allocations.
In some places we use `Vec<Attribute>` and some places we use
`ThinVec<Attribute>` (a.k.a. `AttrVec`). This results in various points
where we have to convert between `Vec` and `ThinVec`.
This commit changes the places that use `Vec<Attribute>` to use
`AttrVec`. A lot of this is mechanical and boring, but there are
some interesting parts:
- It adds a few new methods to `ThinVec`.
- It implements `MapInPlace` for `ThinVec`, and introduces a macro to
avoid the repetition of this trait for `Vec`, `SmallVec`, and
`ThinVec`.
Overall, it makes the code a little nicer, and has little effect on
performance. But it is a precursor to removing
`rustc_data_structures::thin_vec::ThinVec` and replacing it with
`thin_vec::ThinVec`, which is implemented more efficiently.
Don't derive `PartialEq::ne`.
Currently we skip deriving `PartialEq::ne` for C-like (fieldless) enums
and empty structs, thus reyling on the default `ne`. This behaviour is
unnecessarily conservative, because the `PartialEq` docs say this:
> Implementations must ensure that eq and ne are consistent with each other:
>
> `a != b` if and only if `!(a == b)` (ensured by the default
> implementation).
This means that the default implementation (`!(a == b)`) is always good
enough. So this commit changes things such that `ne` is never derived.
The motivation for this change is that not deriving `ne` reduces compile
times and binary sizes.
Observable behaviour may change if a user has defined a type `A` with an
inconsistent `PartialEq` and then defines a type `B` that contains an
`A` and also derives `PartialEq`. Such code is already buggy and
preserving bug-for-bug compatibility isn't necessary.
Two side-effects of the change:
- There is only one error message produced for types where `PartialEq`
cannot be derived, instead of two.
- For coverage reports, some warnings about generated `ne` methods not
being executed have disappeared.
Both side-effects seem fine, and possibly preferable.
Currently we skip deriving `PartialEq::ne` for C-like (fieldless) enums
and empty structs, thus reyling on the default `ne`. This behaviour is
unnecessarily conservative, because the `PartialEq` docs say this:
> Implementations must ensure that eq and ne are consistent with each other:
>
> `a != b` if and only if `!(a == b)` (ensured by the default
> implementation).
This means that the default implementation (`!(a == b)`) is always good
enough. So this commit changes things such that `ne` is never derived.
The motivation for this change is that not deriving `ne` reduces compile
times and binary sizes.
Observable behaviour may change if a user has defined a type `A` with an
inconsistent `PartialEq` and then defines a type `B` that contains an
`A` and also derives `PartialEq`. Such code is already buggy and
preserving bug-for-bug compatibility isn't necessary.
Two side-effects of the change:
- There is only one error message produced for types where `PartialEq`
cannot be derived, instead of two.
- For coverage reports, some warnings about generated `ne` methods not
being executed have disappeared.
Both side-effects seem fine, and possibly preferable.
Currently, for the enums and comparison traits we always check the tag
for equality before doing anything else. This is a bit clumsy. This
commit changes things so that the tags are handled very much like a
zeroth field in the enum.
For `eq`/ne` this makes the code slightly cleaner.
For `partial_cmp` and `cmp` it's a more notable change: in the case
where the tags aren't equal, instead of having a tag equality check
followed by a tag comparison, it just does a single tag comparison.
The commit also improves how `Hash` works for enums: instead of having
duplicated code to hash the tag for every arm within the match, we do
it just once before the match.
All this required replacing the `EnumNonMatchingCollapsed` value with a
new `EnumTag` value.
For fieldless enums the new code is particularly improved. All the code
now produced is close to optimal, being very similar to what you'd write
by hand.
By producing `&T` expressions for fields instead of `T`. This matches
what the existing comments (e.g. on `FieldInfo`) claim is happening, and
it's also what most of the trait-specific code needs.
The exception is `PartialEq`, which needs `T` expressions for lots of
special case error messaging to work. So we now convert the `&T` back to
a `T` for `PartialEq`.
`cs_fold` has four distinct cases, covered by three different function
arguments:
- first field
- combine current field with previous results
- no fields
- non-matching enum variants
This commit clarifies things by replacing the three function arguments
with one that takes a new `CsFold` type with four slightly different)
cases
- single field
- combine result for current field with results for previous fields
- no fields
- non-matching enum variants
This makes the code shorter and clearer.
The deriving code has inconsistent terminology to describe args.
In some places it distinguishes between:
- the `&self` arg (if present), versus
- all other args.
In other places it distinguishes between:
- the `&self` arg (if present) and any other arguments with the same
type (in practice there is at most one, e.g. in `PartialEq::eq`),
versus
- all other args.
The terms "self_args" and "nonself_args" are sometimes used for the
former distinction, and sometimes for the latter. "args" is also
sometimes used for "all other args".
This commit makes the code consistently uses "self_args"/"nonself_args"
for the former and "selflike_args"/"nonselflike_args" for the latter.
This change makes the code easier to read.
The commit also adds a panic on an impossible path (the `Self_` case) in
`extract_arg_details`.
The existing derive code allows for various possibilities that aren't
needed in practice, which complicates the code. There are only a few
auto-derived traits and new ones are unlikely, so this commit simplifies
things.
- `PtrTy` has been eliminated. The `Raw` variant was never used, and the
lifetime for the `Borrowed` variant was always `None`. That left just
the mutability field, which has been inlined as necessary.
- `MethodDef::explicit_self` was a confusing `Option<Option<PtrTy>>`.
Indicating either `&self` or nothing. It's now a `bool`.
- `borrowed_self` is renamed as `self_ref`.
- `Ty::Ptr` is renamed to `Ty::Ref`.
The `&[ast::Variant]` field isn't used.
The `Vec<Ident>` field is only used for its length, but that's always
the same as the length of the `&[Ident]` and so isn't necessary.
