Rollup of 8 pull requests #148454
Closed
Rollup of 8 pull requests #148454
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also, add reference id
When a binding needs to be mutably reborrowed multiple times, suggesting removing `&mut` will lead to follow up errors. Instead suggest both making the binding mutable and removing the reborrow.
```
error[E0596]: cannot borrow `outer` as mutable, as it is not declared as mutable
--> f14.rs:2:12
|
2 | match (&mut outer, 23) {
| ^^^^^^^^^^ cannot borrow as mutable
|
note: the binding is already a mutable borrow
--> f14.rs:1:16
|
1 | fn test(outer: &mut Option<i32>) {
| ^^^^^^^^^^^^^^^^
help: consider making the binding mutable if you need to reborrow multiple times
|
1 | fn test(mut outer: &mut Option<i32>) {
| +++
help: if there is only one mutable reborrow, remove the `&mut`
|
2 - match (&mut outer, 23) {
2 + match (outer, 23) {
|
```
This commit fixes an accidental regression from 144678 where wasm targets would now accidentally use the wrong import module map for a symbol causing a symbol to skip mangling. This can result in compilation failures when symbols are used in cross-crate situations. Closes 148347
```
error[E0401]: can't use `Self` from outer item
--> $DIR/E0401.rs:22:25
|
LL | impl<T> Iterator for A<T> {
| ---- `Self` type implicitly declared here, by this `impl`
...
LL | fn helper(sel: &Self) -> u8 {
| ------ ^^^^ use of `Self` from outer item
| |
| `Self` used in this inner function
|
help: refer to the type directly here instead
|
LL - fn helper(sel: &Self) -> u8 {
LL + fn helper(sel: &A<T>) -> u8 {
|
```
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
…r=davidtwco FCW for repr(C) enums whose discriminant values do not fit into a c_int or c_uint Context: rust-lang#124403 The current behavior of repr(C) enums is as follows: - The discriminant values are interpreted as const expressions of type `isize` - We compute the smallest size that can hold all discriminant values - The target spec contains the smallest size for repr(C) enums - We take the larger of these two sizes Unfortunately, this doesn't always match what C compilers do. In particular, MSVC seems to *always* give enums a size of 4 bytes, whereas the algorithm above will give enums a size of up to 8 bytes on 64bit targets. Here's an example enum affected by this: ``` // We give this size 4 on 32bit targets (with a warning since the discriminant is wrapped to fit an isize) // and size 8 on 64bit targets. #[repr(C)] enum OverflowingEnum { A = 9223372036854775807, // i64::MAX } // MSVC always gives this size 4 (without any warning). // GCC always gives it size 8 (without any warning). // Godbolt: https://godbolt.org/z/P49MaYvMd enum overflowing_enum { OVERFLOWING_ENUM_A = 9223372036854775807, }; ``` If we look at the C standard, then up until C20, there was no official support enums without an explicit underlying type and with discriminants that do not fit an `int`. With C23, this has changed: now enums have to grow automatically if there is an integer type that can hold all their discriminants. MSVC does not implement this part of C23. Furthermore, Rust fundamentally cannot implement this (without major changes)! Enum discriminants work fundamentally different in Rust and C: - In Rust, every enum has a discriminant type entirely determined by its repr flags, and then the discriminant values must be const expressions of that type. For repr(C), that type is `isize`. So from the outset we interpret 9223372036854775807 as an isize literal and never give it a chance to be stored in a bigger type. If the discriminant is given as a literal without type annotation, it gets wrapped implicitly with a warning; otherwise the user has to write `as isize` explicitly and thus trigger the wrapping. Later, we can then decide to make the *tag* that stores the discriminant smaller than the discriminant type if all discriminant values fit into a smaller type, but those values have allready all been made to fit an `isize` so nothing bigger than `isize` could ever come out of this. That makes the behavior of 32bit GCC impossible for us to match. - In C, things flow the other way around: every discriminant value has a type determined entirely by its constant expression, and then the type for the enum is determined based on that. IOW, the expression can have *any type* a priori, different variants can even use a different type, and then the compiler is supposed to look at the resulting *values* (presumably as mathematical integers) and find a type that can hold them all. For the example above, 9223372036854775807 is a signed integer, so the compiler looks for the smallest signed type that can hold it, which is `long long`, and then uses that to compute the size of the enum (at least that's what C23 says should happen and GCC does this correctly). Realistically I think the best we can do is to not attempt to support C23 enums, and to require repr(C) enums to satisfy the C20 requirements: all discriminants must fit into a c_int. So that's what this PR implements, by adding a FCW for enums with discriminants that do not fit into `c_int`. As a slight extension, we do *not* lint enums where all discriminants fit into a `c_uint` (i.e. `unsigned int`): while C20 does (in my reading) not allow this, and C23 does not prescribe the size of such an enum, this seems to behave consistently across compilers (giving the enum the size of an `unsigned int`). IOW, the lint fires whenever our layout algorithm would make the enum larger than an `int`, irrespective of whether we pick a signed or unsigned discriminant. This extension was added because [crater found](rust-lang#147017 (comment)) multiple cases of such enums across the ecosystem. Note that it is impossible to trigger this FCW on targets where isize and c_int are the same size (i.e., the typical 32bit target): since we interpret discriminant values as isize, by the time we look at them, they have already been wrapped. However, we have an existing lint (overflowing_literals) that should notify people when this kind of wrapping occurs implicitly. Also, 64bit targets are much more common. On the other hand, even on 64bit targets it is possible to fall into the same trap by writing a literal that is so big that it does not fit into isize, gets wrapped (triggering overflowing_literals), and the wrapped value fits into c_int. Furthermore, overflowing_literals is just a lint, so if it occurs in a dependency you won't notice. (Arguably there is also a more general problem here: for literals of type `usize`/`isize`, it is fairly easy to write code that only triggers `overflowing_literals` on 32bit targets, and to never see that lint if one develops on a 64bit target.) Specifically, the above example triggers the FCW on 64bit targets, but on 32bit targets we get this err-by-default lint instead (which will be hidden if it occurs in a dependency): ``` error: literal out of range for `isize` --> $DIR/repr-c-big-discriminant1.rs:16:9 | LL | A = 9223372036854775807, | ^^^^^^^^^^^^^^^^^^^ | = note: the literal `9223372036854775807` does not fit into the type `isize` whose range is `-2147483648..=2147483647` = note: `#[deny(overflowing_literals)]` on by default ``` Also see the tests added by this PR. This isn't perfect, but so far I don't think I have seen a better option. In rust-lang#146504 I tried adjusting our enum logic to make the size of the example enum above actually match what C compilers do, but that's a massive breaking change since we have to change the expected type of the discriminant expression from `isize` to `i64` or even `i128` -- so that seems like a no-go. To improve the lint we could analyze things on the HIR level and specifically catch "repr(C) enums with discriminants defined as literals that are too big", but that would have to be on top of the lint in this PR I think since we'd still want to also always check the actually evaluated value (which we can't always determined on the HIR level). Cc `@workingjubilee` `@CAD97`
Suggest making binding `mut` on `&mut` reborrow
When a binding needs to be mutably reborrowed multiple times, suggesting removing `&mut` will lead to follow up errors. Instead suggest both making the binding mutable and removing the reborrow.
```
error[E0596]: cannot borrow `outer` as mutable, as it is not declared as mutable
--> f14.rs:2:12
|
2 | match (&mut outer, 23) {
| ^^^^^^^^^^ cannot borrow as mutable
|
note: the binding is already a mutable borrow
--> f14.rs:1:16
|
1 | fn test(outer: &mut Option<i32>) {
| ^^^^^^^^^^^^^^^^
help: consider making the binding mutable if you need to reborrow multiple times
|
1 | fn test(mut outer: &mut Option<i32>) {
| +++
help: if there is only one mutable reborrow, remove the `&mut`
|
2 - match (&mut outer, 23) {
2 + match (outer, 23) {
|
```
Address rust-lang#81059.
…zelmann Port `cfg!()` macro to the new attribute parsing system r? `@jdonszelmann`
…avidtwco Add check for `+=` typo in let chains Fixes rust-lang#147664 it does affect only cases where variable exist in scope, because if the variable is not exist in scope, the suggestion will not make any sense I wanted to add suggestion for case where variable does not in scope to fix `y += 1` to `let y = 1` but I guess it's too much (not too much work, but too much wild predict of what user wants)? if it's good addition in your opinion I can add this in follow up in other things I guess impl is pretty much self-explanatory, if you see there is some possibilities to improve code or/and some _edge-cases_ that I could overlooked feel free to tell about it ah, also about why I think this change is good and why I originally took it, so it seems to me that this is possible to make this typo (I explained this in comment a little), like, both `+` and `=` is the same button (in most of layouts) and for this reasons I didn't added something like `-=` it seems more harder to make this typo r? diagnostics
…ributes, r=estebank fix: Only special case single line item attribute suggestions `rustc` currently special cases suggestions to add [`#[derive(_)]\n` and other attributes](https://github.com/rust-lang/rust/blob/dc1feabef242259d61bd930713de3250577c1c71/compiler/rustc_errors/src/emitter.rs#L2288C36-L2288C72), to add more context to the suggestions. > // The suggestion adds an entire line of code, ending on a newline, so we'll also > // print the *following* line, to provide context of what we're advising people to > // do. Otherwise you would only see contextless code that can be confused for > // already existing code, despite the colors and UI elements. > // We special case `#[derive(_)]\n` and other attribute suggestions, because those > // are the ones where context is most useful. This special case is a bit broad at the moment and applies to suggestions just to add an attribute, as well as suggestions that contain an attribute and other code, i.e. ```rust #[derive(Clone)] ``` and ```rust #[cfg(not(test))] impl Default for NewWithCfg { fn default() -> Self { Self::new() } } ``` In the latter case, adding a line for context after the suggestion doesn't provide much benefit. Example:  This PR makes it so that this special case only applies to suggestions that just add an attribute and nothing else. This will also make `rustc`'s output match `annotate-snippets`.
reflect that type and const parameter can be intermixed Also, add reference id
…=jackh726 Fix `wasm_import_module` attribute cross-crate This commit fixes an accidental regression from rust-lang#144678 where wasm targets would now accidentally use the wrong import module map for a symbol causing a symbol to skip mangling. This can result in compilation failures when symbols are used in cross-crate situations. Closes rust-lang#148347
Tweak E0401 More accurate span pointing at use place of outer param (at ident instead of full path). Add note explaining why outer item params can't be used in inner item. Use structured suggestion for what `Self` should have been. Follow up to rust-lang#148370. Fix rust-lang#37892.
rustbot
added
A-attributes
|
@bors r+ p=5 rollup=never |
bors
added
S-waiting-on-bors
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
@bors r- |
bors
added
S-waiting-on-author
rustbot
removed
the
S-waiting-on-author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
muton&mutreborrow #147141 (Suggest making bindingmuton&mutreborrow)cfg!()macro to the new attribute parsing system #147945 (Portcfg!()macro to the new attribute parsing system )+=typo in let chains #147951 (Add check for+=typo in let chains)wasm_import_moduleattribute cross-crate #148363 (Fixwasm_import_moduleattribute cross-crate)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup