--- title: Triage meeting 2022-11-29 tags: triage-meeting --- # T-lang meeting agenda * Meeting date: 2022-11-29 ## Attendance * Team members: nikomatsakis, scottmcm, pnkfelix, cramertj * Others: lcnr, compiler-errors, tmandry, nbdd0121, y86-dev, mark, dtolnay ## Meeting roles * Action item scribe: * Note-taker: pnkfelix ## Scheduled meetings - Dec 7: "Contracts and Automated Reasoning for Rust" [lang-team#181](https://github.com/rust-lang/lang-team/issues/181) ## Announcements or custom items ### Tyler Mandry to join lang team *yay* (team distracts itself playing with sound emojis) ## Action item review * [Action items list](https://hackmd.io/gstfhtXYTHa3Jv-P_2RK7A) (team: please async review. Or we can declare bankruptcy if that fails.) ## Pending lang team project proposals None. ## PRs on the lang-team repo None. ## RFCs waiting to be merged ### "Restrictions" rfcs#3323 **Link:** https://github.com/rust-lang/rfcs/pull/3323 josh: For sealed traits and read-only fields josh: syntax isn't perfect, could be revised before stablization. but still seems worth investing exploration effort in. niko: is there an action item for merging this? tyler: is the AI to just hit the merge button? https://forge.rust-lang.org/lang/rfc-merge-procedure.html?highlight=rfc#rfc-merge-procedure niko: Ideally someone would write a bot tyler: sounds nice felix: i will share with tyler what led to my failure to make a bot Tyler to merge ### "Support upcasting of `dyn Trait` values" rfcs#3324 **Link:** https://github.com/rust-lang/rfcs/pull/3324 (Action item on Scott for this, oops) niko: need an unresolved Question or something? scott: I volunteered due to peer pressure last week! what is follow-up? niko: follow-up is writing up concern regarding size of vtables, and add an unresolved Q to be visited during stablization niko: I would personally want to see someone get data, but if no one is motivated to do so, then I'm not too concerned. scott: this is already implemented, right? So impact is already felt? niko: right. Only impact is due to future changes. gary: main motivation against this is the size issue. Perhaps should have lint against combination of this with traits that have multiple supertraits, or against adding new supertraits to existing traits, for stdlib? Maybe a allow-lint in clippy niko: we could have a lint for it, hat seems reasonable. Up to libs team how they want to use it. scott: Is there a comment you can provide with the necessary update here? niko: I can own doing the update itself. scott: okay. I'll merge after you provide the update and ping me. ## Proposed FCPs **Check your boxes!** ### "RFC: `c"…"` string literals" rfcs#3348 - **Link:** https://github.com/rust-lang/rfcs/pull/3348 - [**Tracking Comment**](https://github.com/rust-lang/rfcs/pull/3348#issuecomment-1317925033): > Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: > > * [ ] @cramertj > * [x] @joshtriplett > * [ ] @nikomatsakis > * [ ] @pnkfelix > * [ ] @scottmcm > > No concerns currently listed. > > Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! > > See [this document](https://github.com/rust-lang/rfcbot-rs/blob/master/README.md) for info about what commands tagged team members can give me. - [**Initiating Comment**](https://github.com/rust-lang/rfcs/pull/3348#issuecomment-1317925024): > @rfcbot merge cramertj: Is it already discussed in RFC threads how this interacts with other potential (custom) prefix/suffix features? josh: none other than future work saying "we could add more things like custom ones", but no commitment. scott: we closed the related RFC that was centered around a macro syntax. That's what motivated this instead. cramertj: We've talked about `s"..."` string literals for Strings rather than strs. And other stuff josh: e.g. utf16. and bstr. scott: There are issues around tokenization and validity rules. If we want different tokenization rules for them, then they cannot be user-defined prefix. cramertj: right, full-fledged feature might entail custom delimeters. josh: I can see value of user-defined literals. But if we were going to do that, the path would have been the macro-syntax. We turned that down, at that time. So now we're just talking about adding this one feature, which does not close door on more complete customization feature in future. scott: What I keep coming back to is a type-based thing, where `1` as a token can be many different things depending on type context. scott: We need prefixes to be type-ascribed to different things depending on context. josh: So, something like `"xyz"` becoming `"xyz".fromStringLiteral()` or `fromStringLiteralSomehowPolymorphic!("xyz")`? cramertj: I'm happy to see progress here since it avoids need for defining a `cstr!` macro in my crates. josh: RFC does have note saying that before stablizing this, should have `&CStr` become narrow, or ... (?) niko: This is valuable real estate, I want to make sure its comething people want. cramertj: does have problem that the `c"` prefix is non-googleable scott: We should make such things at least work in the doc-search. We have similar support for other non-googleable things. ### "Stabilize `#![feature(target_feature_11)]`" rust#99767 - **Link:** https://github.com/rust-lang/rust/pull/99767 - [**Tracking Comment**](https://github.com/rust-lang/rust/pull/99767#issuecomment-1320937299): > Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: > > * [ ] @cramertj > * [x] @joshtriplett > * [ ] @nikomatsakis > * [ ] @pnkfelix > * [ ] @scottmcm > > No concerns currently listed. > > Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! > > See [this document](https://github.com/rust-lang/rfcbot-rs/blob/master/README.md) for info about what commands tagged team members can give me. - [**Initiating Comment**](https://github.com/rust-lang/rust/pull/99767#issuecomment-1320937292): > @rfcbot merge scott: might be good to skip forward, since last meeting all time was occupied on proposed RFCs and not on potentially high-priority issues. ### "Remove const eval limit and implement an exponential backoff lint instead" rust#103877 - **Link:** https://github.com/rust-lang/rust/pull/103877 - [**Tracking Comment**](https://github.com/rust-lang/rust/pull/103877#issuecomment-1310223336): > Team member @pnkfelix has proposed to merge this. The next step is review by the rest of the tagged team members: > > * [ ] @cramertj > * [x] @joshtriplett > * [ ] @nikomatsakis > * [x] @pnkfelix > * [ ] @scottmcm > > Concerns: > > * ~~dont-use-lints-for-this~~ resolved by https://github.com/rust-lang/rust/pull/103877#issuecomment-1325306801 > > Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! > > See [this document](https://github.com/rust-lang/rfcbot-rs/blob/master/README.md) for info about what commands tagged team members can give me. - [**Initiating Comment**](https://github.com/rust-lang/rust/pull/103877#issuecomment-1310223314): > Before this change, there was a hard error associated with hitting the internal const-eval limit. And thus there was a notion that every rust compilation is "always" guaranteed to terminate. (Note this was a flawed notion, since proc-macros do not currently have any such limit imposed on their execution.) > > This PR changes that by turning this limit into a lint that can be allowed by the user. > > I want lang team to sign off explicitly on the idea of introducing *some kind* of user-exposed limit that 1. allows the user to change the limit on how long const-eval can proceed (where the values it can change to in this PR are limited to either ∞ via allow or 1,000,000 via deny) and 2. can introduce a new way for the compiler to infinite loop (i.e. by the user turning off the limit) > > Things I want to discourage as being "out of scope" for this fcp discussion: > * This PR also changes the underlying mechanism that drives the diagnostic machinery, namely by using exponential backoff, but I want to treat that as a detail associated with the lint=warn case; the lang team currently should focus solely on the question on whether we expose this as a user-controllable thing at all. In other words, the focus for this fcp should be on the fact that the lint can be set to `allow`. > * The specific details of what metric is used is not under fcp. (@nikomatsakis and I agree that counting terminators executed is not a good metric to expose to users in the long term (it is too coupled to the internal details of MIR).) > * This fcp is not committing us to never exposing a finer-grain control. I.e. it may be a good idea to revise this lint to allow someone to override the threshold, rather than keeping the hard-coded limit of 1,000,000. But I do not want to side-track this fcp with discussions of that; I think exposing the ability to control the specific number should only come after the metric itself has been revisited. > > (I am willing to consider "what should this lints default be: is it allow-by-default or deny-by-default" as something in scope for lang team discussion.) > > @rfcbot fcp merge > > ### "Stabilise inline_const" rust#104087 - **Link:** https://github.com/rust-lang/rust/pull/104087 - [**Tracking Comment**](https://github.com/rust-lang/rust/pull/104087#issuecomment-1315946147): > Team member @scottmcm has proposed to merge this. The next step is review by the rest of the tagged team members: > > * [ ] @cramertj > * [x] @joshtriplett > * [ ] @nikomatsakis > * [ ] @pnkfelix > * [x] @scottmcm > > Concerns: > > * propagate-unsafe (https://github.com/rust-lang/rust/pull/104087#issuecomment-1324173328) > * should-be-ExpressionWithBlock (https://github.com/rust-lang/rust/pull/104087#issuecomment-1324190817) > > Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! > > See [this document](https://github.com/rust-lang/rfcbot-rs/blob/master/README.md) for info about what commands tagged team members can give me. - [**Initiating Comment**](https://github.com/rust-lang/rust/pull/104087#issuecomment-1315946122): > Thanks for the report! It looks like the current state is enough to allow [this to work](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=4c95f0a9ec92f4b1084f4f4021d87393): > ```rust > pub fn create_none_array<T, const N: usize>() -> [Option<T\>; N] { > [const { None }; N] > } > ``` > which is the use that [keeps coming up](https://users.rust-lang.org/t/is-it-possible-to-imply-copy-for-option-none/78892/3?u=scottmcm) on URLO. And this *also* keeps coming up ([1](https://internals.rust-lang.org/t/do-const-fn-s-always-be-inlined/17732/5?u=scottmcm) [2](https://internals.rust-lang.org/t/simple-dependent-types-in-const-contexts/17593/3?u=scottmcm)) on IRLO as needed (or at least wanted) for various things. > > So let's do it! > > @rfcbot fcp merge > > cc Tracking Issue https://github.com/rust-lang/rust/issues/76001, which also covers patterns so isn't closed by this PR. ## Active FCPs None. ## P-critical issues None. ## Nominated RFCs, PRs and issues discussed this meeting ### "Add `core::mem::offset_of!` RFC" rfcs#3308 **Link:** https://github.com/rust-lang/rfcs/pull/3308 josh: looks appealing. scott: doesn't this exist as a crate? one that ralf wrote and fixed to make correct? scott: why not remove lang and just treat it as a T-libs-api question, just a nice wrapper around `addr_of` etc. scott: Here's the crate with Ralf as an author: https://lib.rs/crates/memoffset josh: +1, assuming clear statement that "we know this can be done without language changes, therefore it need not be delivered with our blessing" niko: burntsushi looped us in, because maybe e.g. we might want built-in syntax for this. josh: it was good to be looped in. But I think we're happy to hand off to T-libs-api team. josh: I don't think we necessariy want built-in syntax for this. josh: What is return type? `usize`? gary: it is. felix: is there any interaction with proposal(s) for Fields-in-Traits (FiT)? josh: one proposal said that FiT has to map to a concrete field. another said it can be a property that maps to a function. josh: So perhaps this could use a future work item noting that any FiT effort should account for interactions with `offset_of!`. niko: My proposal was for fields in the struct. Tricky part was accessing via `dyn`, which requires loading. niko: Even if you know the field for a struct exists, you cannot find its offset for `dyn Trait` case without knowing the specific type it maps to. gary: Well you could still find the offsets, from the vtable niko: but then you need the pointer with associated vtable; type isn't enough. josh: If we are going to support detecting that at compile-time, then this might need to be a built-in macro. cramertj: "can't be a macro" just means that compiler-internals are involved, not that the user-visible sytnax cannot be this macro? niko: that's my interpretation. josh posted a comment: https://github.com/rust-lang/rfcs/pull/3308#issuecomment-1331189141 ### "Implement a lint for implicit autoref of raw pointer dereference " rust#103735 **Link:** https://github.com/rust-lang/rust/pull/103735 misc: is this about (`&`) addr-of? no? josh: what is resulting recommended incantation to say "i meant to do this" cramertj: if trying to autoref, you'd need to say `&mut *ptr` niko: `(*ptr).len()` does seem like strong hint that you expect `ptr` to be a valid pointer. quote from PR discussion "if lint as written is too broad, I can imagine restrictions that would be useful" scott: deny-by-default Lint as proposed in the OP seems too strong. niko: the idea is that you should be `&` at the call-site, i.e. instead of `(*ptr).len()`, you would write `(&*ptr).len()` cramertj: What is the case where you write this and meant something else? (Felix missed example) scott: in current miri rules, if you have a (raw) pointer to item 3, you're not allowed to use it to go back and get a pointer to item 2, even if that's what you intended/wanted. gary: PlaceMention, for `let _ = ...`, requires what you are referencing to be valid. This seems like same situation. cramertj: But isn't this an issue specific to `ptr::addr_of!`? E.g. maybe we should warn if any non-place expressions are used inside `ptr::addr_of!` (pnkfelix gets clarification that `ptr::addr_of!` is thing formerly known as `raw_addr_of!`) josh: seems like we should be adding a lint specific to `ptr::addr_of!` and `ptr::addr_of_mut`, not the general case shown here. niko: consensus: this as written is too broad; we would prefer more tailored lint that is specific to things like `ptr::addr_of!` where user is indicating their intention to work with raw pointers. ```rust unsafe fn test_mut(ptr: *mut [u8]) -> *mut [u8] { addr_of_mut!((&mut (*ptr))[..16]) //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } unsafe fn test_mut(ptr: *mut [u8]) -> *mut [u8] { addr_of_mut!((*ptr)[..16]) //~^ warn: implicit auto-ref creates a reference to a dereference of a raw pointer } ``` cramertj: We think that `ptr::addr_of!((*ptr).x)` should be non-UB even for non-valid data behind ptr, right? cramertj: Assuming data behind ptr has to exist scott: Agreed. cramertj: There's no implicit assertion from what I wrote that ptr is derefenceable? scott: There probably is an implication that it is derefernceable. cramertj: Isn't that incompatible with how aforementioned `offset_of!` is implemented? scott: It leverages `MaybeUninit` simulacrum: Pointer has to look like a valid instance of the type you are trying to pull from. gary: doc says "usual rules", but unclear what usual rules are here. E.g. what if I used a dangling pointer? josh: the ask from our team here is that we want the lint to be more narrowly scoped to `addr_of!`. josh: Questions of when `addr_of!` are valid, but not relevant to decision here cramertj: It is relevant; the rules about what is valid are going to dictate what the lint detects/accepts josh: consensus still is that "We should lint on things that are known to be invalid inside `addr_of!`", we can separately investigate what things are known to be valid scott: The `*` can't be the problem because it's really hard to do anything useful with `addr_of!` without a `*`. I think it's more about the creation of a `&` or a `&mut`. niko: re "invaild inside `addr_of!`, we're trying to require that any locations traversed to reach address do not create any new Rust references (outside of what was in input expression), that would then be subject to e.g. Miri's rules cramertj: Crucially, Stacked Borrows state must not be modified as result of evaluating internals of `addr_of!`. niko: I would like to see a nice writeup of my mental model from OpSem team here. tmandry: code sample, is that using Index operator? Why would you `addr_of_mut!` with a range like that? scott: should be a get_unchecked, maybe niko: also odd for line 298, it sounds like lint would warn, but I don't know how to be *more* explicit about my intentions in such cases. scott: I can take action item here to summarize our thoughts, dump as many specific concerns we have, and reach out to Ralf to get feedback from them. cramertj: Does this lint belong in `rustc`, due to propensity for false positives? We have talked about having some policy about this. niko: never got to final policy, but we agree we want very few false warnings, i.e. none. cramertj: I cannot imagine a version of this that doesn't have false positives. scottmcm: I treat policy as "there must be reasonable code you can write to addres the lint, that isn't just slapping on an `#[allow(...)]`" niko: I agree niko: if we say that "the purpose of `addr_of` is to avoid having a reborrow" then it's not a false warning to warn about reborrows in there (as long as there's a way to suppress it) scott: Ralf just opened PR for a `ptr::from_ref` method: "add ptr::from_{ref,mut}" <https://github.com/rust-lang/rust/pull/104977>. scott: So maybe we want a lint that says "that method is actually what you want." niko: Our role is to define what people using `addr_of!` are trying to do. Ralf's role is to write the (narrowly-scoped) lint that detects when their code crosses outside that space. cramertj: because stacked borrows a moving target, if the check is "does this introduce stacked borrows requirements", then it will change as stacked borrows rules change, right? this could cause code that used to be allow to become deny. scottmcm: same as any other lint. cramertj: seems like a case where things might flip back and forth or even subject to some optimization passes -- seems very best effort. ## Nominated RFCs, PRs and issues NOT discussed this meeting ### "RFC: Field projection" rfcs#3318 **Link:** https://github.com/rust-lang/rfcs/pull/3318 ### "impl DispatchFromDyn for Cell and UnsafeCell" rust#97373 **Link:** https://github.com/rust-lang/rust/pull/97373 ### "Panic on invalid usages of `MaybeUninit::uninit().assume_init()`" rust#100423 **Link:** https://github.com/rust-lang/rust/pull/100423 ### "Stabilize default_alloc_error_handler" rust#102318 **Link:** https://github.com/rust-lang/rust/pull/102318 ### "PhantomData: fix documentation wrt interaction with dropck" rust#103413 **Link:** https://github.com/rust-lang/rust/pull/103413 ### "More deriving on packed structs" rust#104429 **Link:** https://github.com/rust-lang/rust/pull/104429 ### "Clearly specify the `instruction_set` inlining restrictions" reference#1307 **Link:** https://github.com/rust-lang/reference/pull/1307