Introduce CoercePointeeWellformed for coherence checks at typeck stage
Fix#135206
This is the first PR to introduce the "wellformedness" check for `derive(CoercePointee)`.
This patch introduces a new error code to cover all the prerequisites of the said macro. The checks that is enforced with this patch is whether the data is indeed `struct` and whether the layout is set to `repr(transparent)`.
A following series of patch will arrive later to address the following concern.
1. #135217 so that we would only admit one single coercion on one type parameter, and leave the rest for future consideration in tandem of development of other coercion rules.
1. Enforcement of data field requirements.
**An open question** is whether there is a good schema to encode the `#[pointee]` as well, so that we could also check if the `#[pointee]` type parameter is indeed `?Sized`.
``@rustbot`` label F-derive_coerce_pointee
includes post-developed commit: do not suggest internal-only keywords as corrections to parse failures.
includes post-developed commit: removed tabs that creeped in into rustfmt tool source code.
includes post-developed commit, placating rustfmt self dogfooding.
includes post-developed commit: add backquotes to prevent markdown checking from trying to treat an attr as a markdown hyperlink/
includes post-developed commit: fix lowering to keep contracts from being erroneously inherited by nested bodies (like closures).
Rebase Conflicts:
- compiler/rustc_parse/src/parser/diagnostics.rs
- compiler/rustc_parse/src/parser/item.rs
- compiler/rustc_span/src/hygiene.rs
Remove contracts keywords from diagnostic messages
Use orphaned error code for the same error it belonged to before.
```
error[E0665]: `#[derive(Default)]` on enum with no `#[default]`
--> $DIR/macros-nonfatal-errors.rs:42:10
|
LL | #[derive(Default)]
| ^^^^^^^
LL | / enum NoDeclaredDefault {
LL | | Foo,
LL | | Bar,
LL | | }
| |_- this enum needs a unit variant marked with `#[default]`
|
= note: this error originates in the derive macro `Default` (in Nightly builds, run with -Z macro-backtrace for more info)
help: make this unit variant default by placing `#[default]` on it
|
LL | #[default] Foo,
| ~~~~~~~~~~~~~~
help: make this unit variant default by placing `#[default]` on it
|
LL | #[default] Bar,
| ~~~~~~~~~~~~~~
```
`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.
Initial implementation of `#[feature(default_field_values]`, proposed in https://github.com/rust-lang/rfcs/pull/3681.
Support default fields in enum struct variant
Allow default values in an enum struct variant definition:
```rust
pub enum Bar {
Foo {
bar: S = S,
baz: i32 = 42 + 3,
}
}
```
Allow using `..` without a base on an enum struct variant
```rust
Bar::Foo { .. }
```
`#[derive(Default)]` doesn't account for these as it is still gating `#[default]` only being allowed on unit variants.
Support `#[derive(Default)]` on enum struct variants with all defaulted fields
```rust
pub enum Bar {
#[default]
Foo {
bar: S = S,
baz: i32 = 42 + 3,
}
}
```
Check for missing fields in typeck instead of mir_build.
Expand test with `const` param case (needs `generic_const_exprs` enabled).
Properly instantiate MIR const
The following works:
```rust
struct S<A> {
a: Vec<A> = Vec::new(),
}
S::<i32> { .. }
```
Add lint for default fields that will always fail const-eval
We *allow* this to happen for API writers that might want to rely on users'
getting a compile error when using the default field, different to the error
that they would get when the field isn't default. We could change this to
*always* error instead of being a lint, if we wanted.
This will *not* catch errors for partially evaluated consts, like when the
expression relies on a const parameter.
Suggestions when encountering `Foo { .. }` without `#[feature(default_field_values)]`:
- Suggest adding a base expression if there are missing fields.
- Suggest enabling the feature if all the missing fields have optional values.
- Suggest removing `..` if there are no missing fields.
Rename macro `SmartPointer` to `CoercePointee`
As per resolution #129104 we will rename the macro to better reflect the technical specification of the feature and clarify the communication.
- `SmartPointer` is renamed to `CoerceReferent`
- `#[pointee]` attribute is renamed to `#[referent]`
- `#![feature(derive_smart_pointer)]` gate is renamed to `#![feature(derive_coerce_referent)]`.
- Any mention of `SmartPointer` in the file names are renamed accordingly.
r? `@compiler-errors`
cc `@nikomatsakis` `@Darksonn`
Retire the `unnamed_fields` feature for now
`#![feature(unnamed_fields)]` was implemented in part in #115131 and #115367, however work on that feature has (afaict) stalled and in the mean time there have been some concerns raised (e.g.[^1][^2]) about whether `unnamed_fields` is worthwhile to have in the language, especially in its current desugaring. Because it represents a compiler implementation burden including a new kind of anonymous ADT and additional complication to field selection, and is quite prone to bugs today, I'm choosing to remove the feature.
However, since I'm not one to really write a bunch of words, I'm specifically *not* going to de-RFC this feature. This PR essentially *rolls back* the state of this feature to "RFC accepted but not yet implemented"; however if anyone wants to formally unapprove the RFC from the t-lang side, then please be my guest. I'm just not totally willing to summarize the various language-facing reasons for why this feature is or is not worthwhile, since I'm coming from the compiler side mostly.
Fixes#117942Fixes#121161Fixes#121263Fixes#121299Fixes#121722Fixes#121799Fixes#126969Fixes#131041
Tracking:
* https://github.com/rust-lang/rust/issues/49804
[^1]: https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unnamed.20struct.2Funion.20fields
[^2]: https://github.com/rust-lang/rust/issues/49804#issuecomment-1972619108
debug-fmt-detail option
I'd like to propose a new option that makes `#[derive(Debug)]` generate no-op implementations that don't print anything, and makes `{:?}` in format strings a no-op.
There are a couple of motivations for this:
1. A more thorough stripping of debug symbols. Binaries stripped of debug symbols still retain some of them through `Debug` implementations. It's hard to avoid that without compiler's help, because debug formatting can be used in many places, including dependencies, and their loggers, asserts, panics, etc.
* In my testing it gives about 2% binary size reduction on top of all other binary-minimizing best practices (including `panic_immediate_abort`). There are targets like Web WASM or embedded where users pay attention to binary sizes.
* Users distributing closed-source binaries may not want to "leak" any symbol names as a matter of principle.
2. Adds ability to test whether code depends on specifics of the `Debug` format implementation in unwise ways (e.g. trying to get data unavailable via public interface, or using it as a serialization format). Because current Rust's debug implementation doesn't change, there's a risk of it becoming a fragile de-facto API that [won't be possible to change in the future](https://www.hyrumslaw.com/). An option that "breaks" it can act as a [grease](https://www.rfc-editor.org/rfc/rfc8701.html).
This implementation is a `-Z fmt-debug=opt` flag that takes:
* `full` — the default, current state.
* `none` — makes derived `Debug` and `{:?}` no-ops. Explicit `impl Debug for T` implementations are left unharmed, but `{:?}` format won't use them, so they may get dead-code eliminated if they aren't invoked directly.
* `shallow` — makes derived `Debug` print only the type's name, without recursing into fields. Fieldless enums print their variant names. `{:?}` works.
The `shallow` option is a compromise between minimizing the `Debug` code, and compatibility. There are popular proc-macro crates that use `Debug::fmt` as a way to convert enum values into their Rust source code.
There's a corresponding `cfg` flag: `#[cfg(fmt_debug = "none")]` that can be used in user code to react to this setting to minimize custom `Debug` implementations or remove unnecessary formatting helper functions.
derive(SmartPointer): assume pointee from the single generic and better error messages
Fix#129465
Actually RFC says that `#[pointee]` can be inferred when there is no ambiguity, or there is only one generic type parameter so to say.
cc ```@Darksonn```
r? ```@compiler-errors```
Expand `for_loops_over_fallibles` lint to lint on fallibles behind references.
Extends the scope of the (warn-by-default) lint `for_loops_over_fallibles` from just `for _ in x` where `x: Option<_>/Result<_, _>` to also cover `x: &(mut) Option<_>/Result<_>`
```rs
fn main() {
// Current lints
for _ in Some(42) {}
for _ in Ok::<_, i32>(42) {}
// New lints
for _ in &Some(42) {}
for _ in &mut Some(42) {}
for _ in &Ok::<_, i32>(42) {}
for _ in &mut Ok::<_, i32>(42) {}
// Should not lint
for _ in Some(42).into_iter() {}
for _ in Some(42).iter() {}
for _ in Some(42).iter_mut() {}
for _ in Ok::<_, i32>(42).into_iter() {}
for _ in Ok::<_, i32>(42).iter() {}
for _ in Ok::<_, i32>(42).iter_mut() {}
}
```
<details><summary><code>cargo build</code> diff</summary>
```diff
diff --git a/old.out b/new.out
index 84215aa..ca195a7 100644
--- a/old.out
+++ b/new.out
`@@` -1,33 +1,93 `@@`
warning: for loop over an `Option`. This is more readably written as an `if let` statement
--> src/main.rs:3:14
|
3 | for _ in Some(42) {}
| ^^^^^^^^
|
= note: `#[warn(for_loops_over_fallibles)]` on by default
help: to check pattern in a loop use `while let`
|
3 | while let Some(_) = Some(42) {}
| ~~~~~~~~~~~~~~~ ~~~
help: consider using `if let` to clear intent
|
3 | if let Some(_) = Some(42) {}
| ~~~~~~~~~~~~ ~~~
warning: for loop over a `Result`. This is more readably written as an `if let` statement
--> src/main.rs:4:14
|
4 | for _ in Ok::<_, i32>(42) {}
| ^^^^^^^^^^^^^^^^
|
help: to check pattern in a loop use `while let`
|
4 | while let Ok(_) = Ok::<_, i32>(42) {}
| ~~~~~~~~~~~~~ ~~~
help: consider using `if let` to clear intent
|
4 | if let Ok(_) = Ok::<_, i32>(42) {}
| ~~~~~~~~~~ ~~~
-warning: `for-loops-over-fallibles` (bin "for-loops-over-fallibles") generated 2 warnings
- Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
+warning: for loop over a `&Option`. This is more readably written as an `if let` statement
+ --> src/main.rs:7:14
+ |
+7 | for _ in &Some(42) {}
+ | ^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+7 | while let Some(_) = &Some(42) {}
+ | ~~~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+7 | if let Some(_) = &Some(42) {}
+ | ~~~~~~~~~~~~ ~~~
+
+warning: for loop over a `&mut Option`. This is more readably written as an `if let` statement
+ --> src/main.rs:8:14
+ |
+8 | for _ in &mut Some(42) {}
+ | ^^^^^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+8 | while let Some(_) = &mut Some(42) {}
+ | ~~~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+8 | if let Some(_) = &mut Some(42) {}
+ | ~~~~~~~~~~~~ ~~~
+
+warning: for loop over a `&Result`. This is more readably written as an `if let` statement
+ --> src/main.rs:9:14
+ |
+9 | for _ in &Ok::<_, i32>(42) {}
+ | ^^^^^^^^^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+9 | while let Ok(_) = &Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+9 | if let Ok(_) = &Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~ ~~~
+
+warning: for loop over a `&mut Result`. This is more readably written as an `if let` statement
+ --> src/main.rs:10:14
+ |
+10 | for _ in &mut Ok::<_, i32>(42) {}
+ | ^^^^^^^^^^^^^^^^^^^^^
+ |
+help: to check pattern in a loop use `while let`
+ |
+10 | while let Ok(_) = &mut Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~~~~ ~~~
+help: consider using `if let` to clear intent
+ |
+10 | if let Ok(_) = &mut Ok::<_, i32>(42) {}
+ | ~~~~~~~~~~ ~~~
+
+warning: `for-loops-over-fallibles` (bin "for-loops-over-fallibles") generated 6 warnings
+ Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
```
</details>
-----
Question:
* ~~Currently, the article `an` is used for `&Option`, and `&mut Option` in the lint diagnostic, since that's what `Option` uses. Is this okay or should it be changed? (likewise, `a` is used for `&Result` and `&mut Result`)~~ The article `a` is used for `&Option`, `&mut Option`, `&Result`, `&mut Result` and (as before) `Result`. Only `Option` uses `an` (as before).
`@rustbot` label +A-lint
Translation of the lint message happens when the actual diagnostic is
created, not when the lint is buffered. Generating the message from
BuiltinLintDiag ensures that all required data to construct the message
is preserved in the LintBuffer, eventually allowing the messages to be
moved to fluent.
Remove the `msg` field from BufferedEarlyLint, it is either generated
from the data in the BuiltinLintDiag or stored inside
BuiltinLintDiag::Normal.
