--- title: Triage meeting 2021-07-13 tags: triage-meeting --- # T-lang meeting agenda * Meeting date: 2021-07-13 ## Attendance * Team members: nikomatsakis, Josh, Taylor * Others: simulacrum ## Meeting roles * Action item scribe: simulacrum * Note-taker: nikomatsakis ## Scheduled meetings - 2021-07-21 / "Lang team process, part 2" [lang-team#104](https://github.com/rust-lang/lang-team/issues/104) - 2021-07-28 / "Structural equality" [lang-team#94](https://github.com/rust-lang/lang-team/issues/94) ## Action item review * [Action items list](https://hackmd.io/gstfhtXYTHa3Jv-P_2RK7A) ## Pending lang team project proposals ### "MCP: Allowing the compiler to eagerly drop values" lang-team#86 **Link:** https://github.com/rust-lang/lang-team/issues/86 * In progress lang-team design note - https://github.com/rust-lang/lang-team/pull/103 * No discussion necessary. ### "negative impls integrated into coherence" lang-team#96 **Link:** https://github.com/rust-lang/lang-team/issues/96 * Need to identify liaision and/or owner. * Niko wanted to discuss at planning meeting, but that didn't happen. ### "Trait Upcasting" lang-team#98 **Link:** https://github.com/rust-lang/lang-team/issues/98 * Discussion point: seconded, but before new initiative process. not clear owner/liaision. May just need to get "merged". ### "Discontinue meeting recordings" lang-team#100 **Link:** https://github.com/rust-lang/lang-team/issues/100 * Needs to be closed -- seems to be no action item? ### "Deprecate target_vendor " lang-team#102 **Link:** https://github.com/rust-lang/lang-team/issues/102 * Not yet seconded; discussion seems in-progress. ## New PRs on the lang-team repo ### "add design notes from lang-team#86 (eager drop)" lang-team#103 **Link:** https://github.com/rust-lang/lang-team/pull/103 ### "introduce new lang team "initiatives" process" lang-team#105 **Link:** https://github.com/rust-lang/lang-team/pull/105 ## RFCs waiting to be merged * joshtriplett: This is a very manual process, can we optimize it a little more? ### "RFC: Overconstraining and omitting `unsafe` in impls of `unsafe` trait methods" rfcs#2316 **Link:** https://github.com/rust-lang/rfcs/pull/2316 ### "RFC: Supertrait item shadowing" rfcs#2845 **Link:** https://github.com/rust-lang/rfcs/pull/2845 ### "`#[derive(Default)]` on enums with a `#[default]` attribute" rfcs#3107 **Link:** https://github.com/rust-lang/rfcs/pull/3107 ### "RFC: let-else statements" rfcs#3137 **Link:** https://github.com/rust-lang/rfcs/pull/3137 - has a concern for the author to add a bit more text to unresolved questions and alternatives ## Proposed FCPs **Check your boxes!** ### "RFC: Add `target` configuration" rfcs#2991 **Link:** https://github.com/rust-lang/rfcs/pull/2991 * Blocked on author of the RFC writing up the alternative syntax. ### "Tracking issue for RFC 2523, `#[cfg(version(..))]`" rust#64796 **Link:** https://github.com/rust-lang/rust/issues/64796 * Blocked on impl of a subset of cfg-accessible. * Related to https://github.com/rust-lang/cargo/pull/9601 * This PR supplied compiler version to build scripts, among a bunch of other things * But author is willing to defer ### "Stabilize `arbitrary_enum_discriminant`" rust#86860 **Link:** https://github.com/rust-lang/rust/pull/86860 * Needs checkboxes. joshtriplett / scottmcm, get on that! ## Active FCPs ### "Add `expr202x` macro pattern" rust#84364 **Link:** https://github.com/rust-lang/rust/pull/84364 ### "Stabilize `const_fn_transmute`, `const_fn_union`" rust#85769 **Link:** https://github.com/rust-lang/rust/pull/85769 * Ralf wanted to make us aware of the implications here * [Thread on Zulip](https://zulip-archive.rust-lang.org/213817tlang/18226Stabilizeconstfntransmuteconstfnunion.html) * Net effect: * This is the first time that you can have non-deterministic const fn (requires unsafe) * in principle you could do it with `const fn foo() -> usize { let x = 5; &x as usize }` too * e.g. by transmuting a pointer to an integer * at runtime: nondeterministic * at compilation time: * today, this function compiles: `const unsafe fn foo() -> usize { let x = 5; std::mem::transmute(&x as *const _) } ` * in general you can pass around pointers that became integers but cannot do much with them * there is some discussion about just how much you should be allowed do with them; e.g., can you mask off and embed data in the low-order bits? * Key point: * In the past, we might've assumed that `const fn` implied purity and determinism, but this is not true given this change. * Now it might be observable * nikomatsakis: Note that the `x as usize` could be unsafe in a const fn, particularly if you take the approach [ralf advocates here](https://www.ralfj.de/blog/2018/07/19/const.html) * joshtriplett: presently, everything that would allow nondeterminism is unsafe; if we go fwd here, we still have the option of maintaining the invariant that nondeterminism requires unsafe * making `as` const (and safe) would be another step ```rust= #![feature(const_fn_transmute)] const unsafe fn foo() -> usize { let x = 5; std::mem::transmute(&x as *const _) } // compiles today ``` ### "Deprecate target_vendor " lang-team#102 **Link:** https://github.com/rust-lang/lang-team/issues/102 ### "introduce new lang team "initiatives" process" lang-team#105 **Link:** https://github.com/rust-lang/lang-team/pull/105 ## T-lang P-critical issues ## Nominated RFCs, PRs and issues ### "Fix how allow/warn/deny/forbid `warnings` is handled" rust#85298 **Link:** https://github.com/rust-lang/rust/pull/85298 * The warnings "lint group" is currently implemented in a surprising way: * Basically, it becomes a kind of "ambient" setting, so that when a lint would *otherwise* be a warning, it gets changed to the level of the warnings group * Downside is: you cannot override it ```rust= #[warn(unused)] #[deny(warnings)] fn main() { #[warn(unused_variables)] let a = 5; // Still gets an error } ``` [yields](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=34bca059e0d41ba8de80200c58384db7) ``` error: unused variable: `a` --> src/main.rs:5:9 | 5 | let a = 5; // Still gets an error | ^ help: if this is intentional, prefix it with an underscore: `_a` | note: the lint level is defined here --> src/main.rs:2:8 | 2 | #[deny(warnings)] | ^^^^^^^^ = note: `#[deny(unused_variables)]` implied by `#[deny(warnings)]` ``` * The PR changes this so that lint level declarations that are "inside" the `warnings` take precedence. * Things that come at the same level, but "later" take precedence ```rust= #[warn(unused)] #[deny(warnings)] // Same depth, greater position (in the attributes list), overrides `warn(unused)` fn main() { #[warn(unused_variables)] // Greater depth, overrides `deny(warnings)` let a = 5; } ``` * Question for *lang team*: * What behavior do we want here? * What backwards compatibility concerns to consider? ```rust= #![warn(some_allow_lint)] #[deny(warnings)] // includes `some_allow_lint` fn foo() { // error if allow_lint trigger, under this PR } ``` ```rust= #![deny(warnings)] #[warn(some_allow_lint)] // warning becomes deny? fn foo() { // warning, under this PR } ``` ```rust= #![deny(some_allow_lint)] #[warn(some_allow_lint)] // warning becomes deny? fn foo() { // warning, today } ``` * cramerj: I would expect `deny(warnings)`, at the crate level, to mean that no warnings would be emitted * scottmcm: if you think of deny warnings as its own thing, that makes sense, but if you think of it as a lint group, not * joshtriplett: the behavior I have seen people expect is that they are searching for the equivalent of `-Werror` from a C compiler; which is to say, if I would ever get a warning, stop and die instead. * We introduce the idea of doing it at a scope level. * But in general the behavior I think people expect is "I don't care where you list it, I want every warning to become an error" * cramertj: this matches my intuition, I have learned to read `deny(warnings)` as `-Werror` * pnkfelix: (double-check: you can do `#[warn(warnings)]` to undo Taylor's hypothesis, right?) [Example](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=eadfde1e2241d2c919fdad35be118e4d) ``` #![deny(warnings)] #[warn(warnings)] fn main() { let x = 5; } ``` yields a warning. * joshtriplett: You could do `-Fwarnings` if you want to prevent that. * cramertj: When I do `Dwarnings` I want it to act like `-Werror` -- that is, unless the lint is *allowed*, I should see an error * (allow is ok because it makes no warning at all) * nikomatsakis: I had never even considered the idea that this was not a group of lints; I think if we want `-Werror` we should add *that* instead * cramertj: yes, I think that is what I would want * simulacrum: also things that are not lints, right? we have some hardcoded warnings * cramertj: I'm thinking of this in "cargo-less" systems for the most part, and I want to ensure that when somebody does a build of a big repository with a bunch of targets that they depend on but don't control, they don't see a bunch of things about code that they don't own * nikomatsakis: isn't that cap-lints? * cramertj: no, I want *hard errors* (min. lint level). I work in a giant monorepo. What happens when I go to build my target is that it is also building a bunch of std libraries maintained by other people I don't know. Their code isn't necessarily logically separated from mine in the build system, so they will receive the same set of compiler flags as my code does, because we don't have a distinction between internal/external dependencies. What I *want* is to ensure that any code that lands in that repository is not giving warnings or command line messages to downstream consumers. * That means: if it would've triggered a lint, it gets fixed before it goes in. * Once it's in, I want people to never see any warnings on the command line because there are no warnings. * joshtriplett: this matches my intuition as well. It had never occurred to me that somebody could allow warnings. * Makes me tempted to forbid. * cramertj: that's too strong, because sometimes you want to allow specific warnings. * joshtriplett: I thought we had a conversation a long time back about how forbid and lint groups behaves. I thought that if you forbid a group, it doesn't necessarily mean that you forbid each lint? I can't remember. * (nobody can remember) * in any case, I would like the ability to say "I want all warnings to be errors" and "I can still override individual warnings by name" * joshtriplett: I think that based on the behavior cramertj and I expect, that would argue for closing this PR? * nikomatsakis: It's not quite, right? It's neither at present. * cramertj: I think the outcome was that we should think of `warnings` as a lint group...? * simulacrum: but it works today and a lot of people rely on it * pnkfelix: https://github.com/rust-lang/rust/issues/81218 seems like the issue that josh was talking about * seems like we didn't come to any concrete conclusions * pnkfelix: https://github.com/rust-lang/rust/pull/81556 : we decided to Future-compatibility-warn on a `forbid(group)` followed by `allow(indvidual)`. * nikomatsakis: I am surprised you all thought it was a mode, but I think right now it's like half a lint group and half a mode, and I think we should make it be one or the other. We might want to disallow e.g. `#[warn(foo)]`. * joshtriplett: what is the semantics/use case for it acts like a lint group? * scottmcm: I would expect it to act "as if" you had typed the names of the lints; I would think it would work like the other lint groups that is a pre-defined set, and warnings it short-hand for the "things that say will warn by default". * nikomatsakis: that is what I would've expected, but I am ok with saying "Warnings is not a lint group, don't think of it that way", just to avoid churn. I don't really have a use case for this. But then we should clarify and make it do that job better. * pnkfelix: my intuition is that warnings as a "normal lint group" would side-step a lot of thorny semantic questions, and could add separate flag to get what Taylor et al want. * If we didn't take that approach, though, and we left the current behavior, we could lint on `allow(warnings)` or `warn(warnings)`. * cramertj: wait, is this lint warn? * pnkfelix: * cramertj: * pnkfelix: * C-c * joshtriplett: I think documenting current behavior and warning where it diverges from `-Werror` would be good * 1) Warn if you `#[warn(warnings)]` or `#[allow(warnings)]` when `#[deny(warnings)]` is active. * This will actually error unless `-Acap-lints` is involved... * 2) Warn if you `#[warn(lint)]` (on a warn or deny lint, not an allow lint) inside of a `#[deny(warnings)]`. (Note that people almost never seem to `#[warn(x)]`.) * simulacrum: the second case, putting `#[warn]` inside of `#[deny(warnings)]`, is something that rustc relies upon as a feature * josh: Then given that people almost never do it otherwise "in the wild", we don't need to worry about warning on it * pnkfelix: we could do a FCW for case (1), right? * nikomatsakis: but why? no soundness reason? * pnkfelix: If we don't warn about (2), wouldn't cramertj not get his property? * simulacrum: second case will issue an error today, so cramertj is happy * joshtriplett: I would only do (2) specifically for the case where `lint` was deny-by-default * this is the case that doesn't behave like a lint group * The user was trying, presumably, to "downgrade" from a deny-to-a-warn * nikomatsakis: I would prefer not to do (2) because it disables a potential use case * It seems like the use case is "issue warnings" when I'm building locally, and hard errors on CI * And that might be what people wanted for the deny-by-default lint too * Consensus: * Close this PR, working as intended * Request documentation * simulacrum: wait, I use `allow(warnings)` all the time * nikomatsakis: oh, yes, so do I, but in that case the behavior doesn't match what I would want: * scottmcm: Hmm, a place where the "it's a lint group" might be valuable is `#![allow(warnings)]` on the crate but `#[warn(warnings)]` again on a particular method. (Not that I think I'd ever recommend that.) ``` #![allow(warnings)] #[warn(unused_variable)] fn foo() { let x = 3; // This doesn't warn (I checked) and that's weird (in scott's opinion, because of the scoping) // counterpoint: simulacrum thinks this shouldn't warn indeed } ``` * cramertj: It might be that there was a bunch of warnings and you added `#![allow(warnings)]` to make it stop ### "Allow limited transmuting between types involving type parameters" rust#86281 **Link:** https://github.com/rust-lang/rust/pull/86281 OP: > Before this change, functions could never perform transmutes between types which involve type parameters. > > This opens up slightly less limited analysis whereby certain types involving `Sized` type parameters can be considered relative to each other, even if the exact size of that type parameter isn't yet known because monomorphization hasn't happened yet. Specifically it allows: > 1. `repr(transparent)` types to be considered to have the same size as the `Sized` type they wrap. > 2. Arrays of type parameters (or `repr(transaprent)` wrappers thereof) to be considered to have the same size as other arrays of the same type parameters (or `repr(transparent)` wrappers thereof), as long as their element count is the same. > > It only supports very limited comparisons, but it opens up more safe and legal transmutes than were previously allowed. > > In particular, it allows for transmutes like: > ```rust > fn make_array<T, const N: usize>() -> [T; N] { > let mut values: [MaybeUninit<T>; N] = unsafe { MaybeUninit::<[MaybeUninit<T>; N]>::uninit().assume_init() }; > // [... Init values] > std::mem::transmute(values) > } > ``` > which are currently not allowed because `[T; N]` isn't considered to have the same size as `[MaybeUninit<T>; N]` > > Fixes #61956. scottmcm comments: > Personally I'm torn here. On one hand, I think this would be quite useful. On the other hand, I worry that this might make it harder to make transmute non-magic. > > Maybe someone from the const WG has an idea about whether this would make it harder to encode the "sizes are the same" check as a "real" type system predicate instead of intrinsic magic? > > For the specific example in the OP, one way forward would be to just stabilize https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.array_assume_init (or something like it). ### "Associated functions that contain extern indicator or have `#[rustc_std_internal_symbol]` are reachable" rust#86492 **Link:** https://github.com/rust-lang/rust/pull/86492 * [Discussed in previous meeting](https://github.com/rust-lang/lang-team/blob/master/minutes/2021-07-06.md#associated-functions-that-contain-extern-indicator-or-have-rustc_std_internal_symbol-are-reachable-rust86492) * TL;DR * `#[no_mangle]` on an inherent method currently is not considered reachable, but is allowed * Clear that we have a couple of stabs in a larger design space * Options: * Support `#[no_mangle]` on inherent methods; symbol has name of method; make it be considered reachable * Disallow it, permit only on top-level functions, and address more broadly * No real loss of expressiveness either way ### "Fix autoborrowing when coercing to a mutable raw pointer" rust#86647 **Link:** https://github.com/rust-lang/rust/pull/86647 From last meeting: > Discussed in the @rust-lang/lang team meeting today. We weren't really sure what is being proposed here! @scottmcm is going to follow up and try to figure out how to describe the effects of this proposed PR on the language itself. ### "Allow reifying intrinsics to `fn` pointers." rust#86699 **Link:** https://github.com/rust-lang/rust/pull/86699 Niko summarized: > I'm tagging this as T-lang and nominating, as it seems like a lang change. It'd perhaps be nice to write up a lang proposal or something that summarizes it. It's a short PR. The details are: > > * The type of an intrinsic like `transmute` is a zero-sized function type. > * Under this PR, it can be coerce to an `extern "Rust"` function pointer > * This will create a wrapper around the intrinsic. > * This mechanism is 'insta-stable', but the only stable intrinsic is believed to be `transmute`. ### "Allow labeled loops as value expressions for `break`" rust#87026 **Link:** https://github.com/rust-lang/rust/pull/87026 > Nominated for @rust-lang/lang. I believe what is being proposed here is a change to the grammar. I believe the grammar would be something like this: > > ``` > BreakExpr := > 'break' > 'break' lifetime Expr > 'break' Expr > > Expr := > lifetime: 'loop' > ... > ``` > > In other words, you can write `break 'foo: loop { .. }`. I would sort of expect this to work but I guess it didn't, presumably because of a parser bug. (I've not read the code closely.) > > Does this sound correct? scottmcm adds: > Given that this works with `return` already, I'd personally consider this a bug, and that it was probably expected to work this way the whole time. > > (It seems like it's an easy mistake to make, since `return try { ... }` didn't use to work either, https://github.com/rust-lang/rust/pull/76274 .) ### "Support `#[track_caller]` on closures and generators" rust#87064 **Link:** https://github.com/rust-lang/rust/pull/87064 > This PR allows applying a `#[track_caller]` attribute to a > closure/generator expression. The attribute as interpreted as applying > to the compiler-generated implementation of the corresponding trait > method (`FnOnce::call_once`, `FnMut::call_mut`, `Fn::call`, or > `Generator::resume`). > > This feature does not have its own feature gate - however, it requires > `#![feature(stmt_expr_attributes)]` in order to actually apply > an attribute to a closure or generator. > > This is implemented in the same way as for functions - an extra > location argument is appended to the end of the ABI. For closures, > this argument is *not* part of the 'tupled' argument storing the > parameters - the final closure argument for `#[track_caller]` closures > is no longer a tuple. > > For direct (monomorphized) calls, the necessary support was already > implemented - we just needeed to adjust some assertions around checking > the ABI and argument count to take closures into account. > > For calls through a trait object, more work was needed. > When creating a `ReifyShim`, we need to create a shim > for the trait method (e.g. `FnOnce::call_mut`) - unlike normal > functions, closures are never invoked directly, and always go through a > trait method. > > Additional handling was needed for `InstanceDef::ClosureOnceShim`. In > order to pass location information throgh a direct (monomorphized) call > to `FnOnce::call_once` on an `FnMut` closure, we need to make > `ClosureOnceShim` aware of `#[tracked_caller]`. A new field > `track_caller` is added to `ClosureOnceShim` - this is used by > `InstanceDef::requires_caller` location, allowing codegen to > pass through the extra location argument. > > Since `ClosureOnceShim.track_caller` is only used by codegen, > we end up generating two identical MIR shims - one for > `track_caller == true`, and one for `track_caller == false`. However, > these two shims are used by the entire crate (i.e. it's two shims total, > not two shims per unique closure), so this shouldn't a big deal. ### "Clarify "string continue" for (byte) string literals" reference#1042 **Link:** https://github.com/rust-lang/reference/pull/1042 > The previous version just said "whitespace at the beginning of the next > line is ignored", but that is not quite correct. Currently, exactly four > characters are ignored in that position. This is different from the > definition of `char::is_whitespace` and `char::is_ascii_whitespace`. > > Additionally "at the beginning of the next line" is confusing as > additional \n are also ignored. > > See the lexer implementation: > > https://github.com/rust-lang/rust/blob/595088d602049d821bf9a217f2d79aea40715208/compiler/rustc_lexer/src/unescape.rs#L281-L287 > > https://github.com/rust-lang/rust/blob/595088d602049d821bf9a217f2d79aea40715208/compiler/rustc_lexer/src/unescape.rs#L300-L307