None.
Edit the schedule here: https://github.com/orgs/rust-lang/projects/31/views/7.
(Meeting attendees, feel free to add items here!)
TC: For any guests who are present, please note in this section if you're attending for the purposes of any items on (or off) the agenda in particular.
TC: As we've been doing recently, due to the impressive backlog, I'm going to push the pace a bit. If it's ever too fast or you need a moment before we move on, please raise a hand and we'll pause.
TC: Remember that we have a design/planning meeting that starts half an hour after this call ends.
TC: We had three meetings with the RfL team, on 2024-02-12, 2024-02-21, and 2024-03-06. We haven't met since then. We had planned to meet every two weeks (but there had been some reservations on our side about that frequency even at the outset).
They've since reached back out.
Perhaps a good topic would about project goals for RfL. Other topics that have been mentioned include:
derive(SmartPointer)
If we have substantial questions on #124323 (which is nominated), the stabilization of a new target feature for aarch64, we could also discuss that.
NM: I'd like to put it for a time when we can review something concrete. Maybe two weeks from today.
tmandry: We may need an owner for a more general goal around SmartPointer
.
NM: I was going to propose something more tightly scoped around intrusive linked list support. That's the theme. The next step would be the smart pointer work. Then fallible allocation is another goal. Where I'd like feedback from them is, first, can we find an owner. Then next, to confirm the priorities. I do think we want a write-up somewhere.
TC: OK, we'll do it two weeks from today. I'll let Miguel know.
TC: Note that Niko now has a draft RFC on RTN:
https://hackmd.io/KJaC_dhZTmyR_Ja9ghdZvg
With discussion here:
https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/return-type.20notation
Both tmandry and I have reviewed it. Everyone is hoping to get this posted soon.
Project board: https://github.com/orgs/rust-lang/projects/43/views/5
None.
TC: We have tracking issues for the Rust 2024 aspects of every item queued for the edition:
https://github.com/rust-lang/rust/issues?q=label%3AA-edition-2024+label%3AC-tracking-issue
For each item, we've identified an owner. Our most recent update for item owners is here:
https://rust-lang.zulipchat.com/#narrow/stream/268952-edition/topic/Owners.20update.202024-04-30
Our motivating priorities are:
We're coming up on the 1 May deadline for "code complete" that had originally been set. This will help to give us some idea of where things are. Note, however, I've talked with many highly plugged-in people who until 2-4 weeks ago thought this timeline would be much later (e.g. October) and hadn't heard any differently. This probably was not their fault. We're reviewing the state of things and analyzing what may need to happen in terms of this timeline. We want to carefully balance all factors. I'll write more about this in the coming days.
NM: What's the timeline you're tentatively thinking about? I could see either "moderate" or "long" pushback, where "moderate" would be releasing Rust 2024 in early 2025, and "long" would be something further, e.g. Rust 2025.
TC: We need to carefully review, but probably cutting the beta for Rust 2024 at the end of the year or just after and letting it ride the train for release in February. We'll call it Rust 2024.
NM: That sounds reasonable to me.
Link: https://github.com/rust-lang/rust/issues/117587
TC: With the acceptance of RFC 3617 and the great work by CE, this is looking to be in good shape for the edition.
Link: https://github.com/rust-lang/rfcs/pull/3513
TC: With the acceptance of RFC 3513 and the great work by Oli, this is looking to be in good shape for the edition.
!
to a type (RFC 1216) #35121Link: https://github.com/rust-lang/rust/issues/35121
Link: https://github.com/rust-lang/rust/pull/123508
TC: We FCPed a plan for this and Waffle is moving things along. I've been checking in. Looks good so far.
Link: https://github.com/rust-lang/rust/issues/124509
TC: Automatic differentiation is the ability to turn an arbitrary function like this:
#![feature(autodiff)]
#[autodiff(df)]
fn f(x: f32) -> f32 { x * x }
…into one like this:
fn df(x: f32) -> (f32, f32) {
(x * x, 2 * x)
}
That is, remembering our calculus for a moment:
f(x) := x^2
f'(x) := 2*x
Manuel Drehwald and his collaborators have been hard at work for a long time now to bring this magical power to Rust. This is built on an underlying technology (also of their doing) called EnzymeAD that does these transforms on the LLVM IR.
They're now ready to merge this work into nightly Rust as an experimental feature. On the compiler side, this was approved as an MCP:
It's now to us to approve a lang experiment. This will include, for now, the #[autodiff]
attribute macro (there are other syntaxes, of course, that may be worthy of experimentation). The goal of the experiment is to better inform the RFC and the other design documents that are currently in the works. Merging this work will help Manuel and his collaborates move these experiments forward, as maintaining an out-of-tree branch of this scale, as they've been doing, is painful.
Manuel has put together an impressive list of researchers who have expressed support for and interest in this work. We've talked about wanting to get more academic interest in Rust, and this could be one way to do that. He's also been working with RalfJ on aspects of this.
Oli is mentoring this on the compiler side, and I've been working with Manuel for many months and shepherding this toward what is and will be needed on the lang side (e.g. design documents, RFC, etc.). It may make sense for me to liaison here (though I understand scottmcm has had some discussions with Manuel also).
TC: Are we OK with this going forward as an experiment? (scottmcm, do you want to liaison, or should I take this?)
scottmcm: They've clearly done a lot of work. It seems fine on the lang side. I'm curious what this means on the compiler side.
Josh: I'm curious what the value is in merging with us if it's a fork on the LLVM side?
TC: They're working on merging on the LLVM side also.
Josh: Is there a bootstrap problem; they need to see traction elsewhere to merge it there?
TC: There may be some of that.
tmandry: I'm curious what this means for our backend story.
scottmcm: Yes, this would be LLVM specific. I don't know how we feel about that.
Josh: To be clear, I would not propoose a policy that we block LLVM specific things. E.g., I suspect in the future that we may have a GPU backend.
NM: Agree with that. I propose that we separate out what we need to do to approve the experiment. I propose we talk about how we support backend-specific features rather than that should we. We're going to need to do this. I have a talking point I'm toying with, e.g. "leave nothing on the table." It would take something away from using a widely-used compiler toolchain if we didn't take advantage of what that offered.
Josh: Agreed. I don't think we want to exclusively allow Rust features that can always work on every compiler backend (which would mean reimplementing them in rustc), nor should we blithely add numerous extensions that exist in one backend. There is a healthy balance here somewhere. I suspect the balance skews toward mostly allowing backend-independent features but allowing a small handful of backend-specific features. And if someone wants one of those to work on another backend they should then expend effort reimplementing in rustc so it works on every backend.
pnkfelix: Since this uses a branch of LLVM, how would this affect people building it?
(Discussion that this may be more like a module.)
Manuel: We don't rely on anything not in LLVM. We have some extensions to they're analysis that we'd like to merge, but those aren't required.
Manuel: Enzyme supports a large range of LLVM versions, so right now we just add Enzyme as an additional library during bootstrap, to which we then pass LLVM Modules during compile time. We support LLVM 11 to head.
scottmcm: What I was concerned about is not approving this as an experiment but then later telling them that this would have to be done in a totally different way. It doesn't sound like that's the case. So it seems like we can approve this on the lang side and let compiler do what they need to do.
NM: The compiler team MCP, as I understand it, is them taking responsibility to the part of the user experience on how people compile Rust. That part probably isn't our job. Though it is good for us to know about this. Also, this is more a stabilization question.
NM: I'd like to hear also about what APIs would be required for this to not need to be a compiler built-in.
Josh: I would like this to be marked with the experimental flag (even after an RFC is accepted).
tmandry: I agree we shouldn't block this.
NM proposed consensus:
Consensus: We're OK with this going forward as an experiment and TC will liaison.
Link: https://github.com/rust-lang/rust/pull/123590
TC: The motivation for this PR is that if we're going to stabilize the never type, perhaps we should make it less weird; this is one way to do that.
Normally when a block ends in ;
its type is ()
. E.g.:
let _: () = {
();
};
However, if the block is known to diverge, we have a special case:
let _: ! = {
unsafe { unreachable_unchecked() };
();
};
While this makes some intuitive sense to us as humans, we don't generally let control-flow analysis bleed into type checking, and, e.g., this does not work:
let _: ! = {
if true {
unsafe { unreachable_unchecked() };
}
();
};
//~^ ERROR mismatched types
If we remove the special case, the fix is always to remove the semicolon and any dead code that follows. There is already a machine-applicable fix for this.
In discussion, people have seemed to think that, fundamentally, this change makes sense and that if we had a time machine, we should go back and do it this way. The remaining questions are practical.
The first thing we discussed was how to not make this a hard edge between editions inclusive of rustfmt
behavior. We wanted it to be possible to write code, that when run through rustfmt
, would work in both Rust 2021 and Rust 2024. This will be solved by:
Next, we discussed the question of how much churn this induces and whether it's a conceivable amount for Rust 2024 or whether we should consider it for a later edition. To help gauge this, we asked to see the standard library migrated. The results of this can be seen here:
Waffle notes that almost all changes were handled by the automation. The remaining cases tended to be helper functions that followed an explicit return. In these cases, the helper functions needed to be moved earlier in the body. There weren't a lot of these given the size of the standard library.
Here are some options I see for moving forward:
return
(and perhaps break
and continue
) are used literally.
return
in front.TC: NM has proposed this for FCP. This is edition-sensitive. What do we think?
tmandry: Because we've been telling people, via rustfmt
, to put a semicolon after return
, it makes me concerned about accepting this as-is.
Josh: Maybe it makes sense to special case the very last item but not things in the middle of the block. That seems like it would work for almost every case in the standard library.
scottmcm: Prepping some examples:
I really like these:
pub fn abort() -> ! {
- crate::sys::abort_internal();
+ crate::sys::abort_internal()
}
let Some(id) = last.checked_add(1) else {
- exhausted();
+ exhausted()
};
This feels awkward:
} else {
- return false; // other is empty
+ return false // other is empty
};
scottmcm: The exhausted
cause feels compelling to me along with the first one. This gets to the point about local reasoning. These do feel much more reasonable to me and are worth taking some churn.
scottmcm: The third one, though, there are a lot of those. That feels like maybe a lot of churn. So maybe I'd like the option 2 above. We could special case it syntactically diverging, as long as there's nothing after it.
Niko: scottmcm, what about this one…
else {
panic!("...");
}
scottmcm: That's interesting because the macro could expand to a return
. I'm not sure how I feel about that one.
nikomatsakis: …in fact it appears to be built-in.
NM: When I talked with Rust users in the early days, people found the rules for when to add a semicolon were things people found confusing. I worry about us swimming upstream if we did this for return
. For panic
, I tend to think it's the same. But I don't know there. I second the sense of others here. It seems from the diff that return
is 80 percent or more.
tmandry: This would remove the ability to have a return followed by an inner function, which is something I've used before.
NM: That actually just seems like a bug to me. I wonder if we could just fix that. You would have "statements; expr items" essentially.
tmandry: That does seem desirable. Error recovery may be an issue there.
Josh: Agreed on that. The human reader could find it confusing also. But I'd probably never write inline functions except at the top of the function.
NM: Conversely, I find it confusing the other way, it seems to be leaking compiler details and doesn't follow how my brain thinks about the "grammar" (I also find inline functions at the top kind of hard to read, so clearly varies depending on the individual).
Examples:
Rustc accepts
{
return // or break, continue
}
{
return; // or break, continue
}
{
panic!(...); // or unimplemented, todo, etc?
}
{
function_that_returns_never()
}
Independently, maybe we want to accept these?
fn foo() -> ! {
$tail_expr
fn foo() { } // or other items, e.g., `const C: u32 = ...;
}
fn foo() -> ! {
return;
fn foo() { } // or other items, e.g., `const C: u32 = ...;
}
fn foo() -> ! {
function_that_returns_never()
fn foo() { } // or other items, e.g., `const C: u32 = ...;`
}
Rustc does NOT accept (in new edition)
fn foo() -> ! {
return;
other_stuff();
}
fn foo() -> ! {
function_that_returns_never();
}
fn foo() -> ! {
function_that_returns_never();
other_stuff();
}
Josh: It does seem good to narrow the special case as far as possible. Maybe just to return
, break
, continue
, and the obvious macros.
scottmcm: Yes, we could look transparently through macros, and have those macros expand to return something_that_diverges()
.
Josh: +1 as long as we don't incur a performance hit for changing the expansion of common macros. (We could perhaps ameliorate such a performance hit with an attr on the macro instead of a change to the expansion.)
scottmcm: Note that rustfmt will stop touching the semicolons going forward. This is a great long-term answer for what rustfmt should do.
tmandry: I like rustfmt being opinionated and doing the right thing here.
TC/scottmcm: It doesn't have type checking information, so it can be limited in these cases.
NM: It would be good if we could make it so that rustfmt could do the right thing.
Summary: There seems to be a tentative consensus forming around option 2 where we do the special-casing only on return
, continue
, and break
, at type-checking time and after macro expansion. We'll look at this again after answering the cases that Niko laid out above.
(The first part of the meeting ended here.)
We're continuing triage in the remainder of our planning meeting.
Attendees: TC, NM, pnkfelix, tmandry, Josh, eholk, scottmcm
non-local-definitions
lint" rust#124396Link: https://github.com/rust-lang/rust/issues/124396
TC: In RFC 3373, we decided to lint against cases like this:
trait Trait {}
struct Ty {}
fn foo() {
impl Trait for Ty {}
//~^ WARN non-local `impl` definition, they should be avoided
//~| as they go against expectation
}
These are "sneaky inner impls" since the inner impl has a non-local effect.
(T-types later had a bright idea about how to implement this lint, and that was done here: https://github.com/rust-lang/rust/pull/122747)
However, someone recently noticed a class of false positives. They look like this:
trait Trait {}
fn foo() {
mod m {
pub struct Ty {}
}
impl Trait for m::Ty {}
//~^ WARN non-local `impl` definition, they should be avoided
//~| as they go against expectation
}
This is not a sneaky inner impl, just as it wouldn't be if that module were not there.
The proposed fix is to make the "modules transparent when checking if the impl
def is at the same nesting as the" type or trait. This is similar to the const _: () = { .. }
exception. Urgau has checked with the r-a folks and this works for them. Josh and I both think it's OK. Nobody can think of how create any problems and it seems in the spirit of the RFC.
This needs to be beta-backported, so Urgau is looking for feedback soon.
Urgau: lncr also seems to agrees and has already merged the PR on nightly (https://github.com/rust-lang/rust/pull/124539)
TC: Do we agree?
NM: Sounds fine. The same nesting means in the same function item?
Josh: Roughly, yes. The type or the trait needs to not be more visible than the impl. (Handwaving, types has a more precise statement of this.)
tmandry: Sounds fine.
Consensus: This is OK by us. Also, lang does not have to approve beta-backports.
#[repr(C)]
enums." rust#123043Link: https://github.com/rust-lang/rust/pull/123043
As background, from GoldsteinE:
Consider this enum:
#[repr(C)]
enum Fun<T> {
Fun(T),
// maybe some other variants here
}
For unsafe code it could be useful to assume that
Fun<T>
is layout-compatible withFun<MaybeUninit<T>>
for any choice ofT
. That’s not currently the case: ifT
is uninhabited,Fun<T>
would be zero-sized (or, more generally, the::Fun
variant will not affect enum size), butFun<MaybeUninit<T>>
wouldn’t.Reference currently documents the above enum as being rough equivalent to something like
#[repr(C)]
enum FunDiscriminant {
Fun,
}
#[repr(C)]
union FunData<T> {
fun: T,
}
#[repr(C)]
struct Fun<T> {
discriminant: FunDiscriminant,
data: FunData<T>,
}
That doesn’t hold for uninhabited types: this “desugaring” preserves the invariant above.
I think we should do one of the following:
- Either dead variant removal should be disabled for
#[repr(C)]
enums;- or it should be documented in the reference (I can write the patch if this option is selected).
Playground demonstrating the issue: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d2995ff99cea9c2e60d28cbf0f970176
He then proposes for us this PR:
This prevents removing dead branches from a
#[repr(C)]
enum (they now get discriminants allocated as if they were inhabited).Implementation notes: ABI of something like
#[repr(C)]
enum Foo {
Foo(!),
}
is still
Uninhabited
, but its layout is now computed as if all the branches were inhabited. This seemed to me like a proper way to do it, especially given that ABI sanity check explicitly asserts that type-level uninhabitedness implies ABI uninhabitedness.This probably needs some sort of FCP (given that it changes
#[repr(C)]
layout, which is a stable guarantee), but I’m not sure how to call for one or which team is the most relevant.
TC: What do we think?
NM: This seems like it's not changing the reference, but fixing a bug, in which case I'm in favor. The set of variants that are listed in the enum are about layout.
pnkfelix: I agree.
tmandry: It seems like a bug fix. Sounds good to me.
Consensus: We're OK with this and believe it to be a bug fix. Therefore, no FCP needed and it's OK to go ahead based on our meeting consensus.
Link: https://github.com/rust-lang/rust/issues/123281
TC: We unanimously FCPed a decision to accept a stable-to-nightly regression as a consequence of one of our earlier decisions:
https://github.com/rust-lang/rust/issues/121250
The net result looks like this:
[INFO] [stdout] error[E0492]: constants cannot refer to interior mutable data
[INFO] [stdout] --> src/lib.rs:583:43
[INFO] [stdout] |
[INFO] [stdout] 583 | const VTABLE: &'static Self::VTable = &(*Lhs::VTABLE, *Rhs::VTABLE);
[INFO] [stdout] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this borrow of an interior mutable value may end up in the final value
[INFO] [stdout]
The current issue was nominated for us by apiraino who wants an opinion from us about whether this should be included in the release notes.
Josh: Anytime we're making a compatibility change, we should flag it, and we can make a note that we don't think it affects anyone based on crater runs.
NM/tmandry/TC: +1.
Consensus: Let's let's flag it.
REDUNDANT_IMPORTS
lint for new redundant import detection" rust#123813Link: https://github.com/rust-lang/rust/pull/123813
TC: We expanded the unused_imports
lint to fire for redundant imports.
This flagged more code in practice than expected, causing us to notice this "fallout".
This and our discussion prompted tmandry to draft a new policy for lint expansions which is separately nominated.
On the original issue, Josh asked for the lint expansion to be reverted and asked for a lang second. That didn't happen, but CE went ahead and reverted the lint.
CE now proposes to reintroduce the lint under a new name, redundant_imports
(to separate it from unused_imports
). He suggests that this lint should be warn-by-default at the end of the day, and asks how we would like to get there.
In discussion, CE and I talked about how, on the one hand, there is churn from this lint firing, but on the other hand, there are people who are particular about code quality and have set #![deny(unused)]
and who probably want this lint to fire for their code, and so we may be asking these people to add another deny
to achieve their goals unless we include this in some existing lint group.
Josh: This came up in part because, in libs-api, we want to add some new functions to the prelude. While we could do it in edition, we could add them in every prelude. But this lint would be a problem for that.
TC: That is a serious problem. Should we make a blanket exception here for wildcard imorts?
Josh: That seems like a reasonable proposal.
NM: There's another reason to like it. If you want to incrementally move away from a glob important, it would be a problem here also.
Josh: That is a great proposal. If that can be reasonably done, we should maybe do that.
tmandry: We could split out the one for glob imports, and make that allow-by-default.
tmandry: It seems this would be less disruptive with the glob import case split out.
NM: I'd like to keep the freedom to expand our definition of cleanliness, but there are tradeoffs here.
Consensus: Let's make an exception for wildcard imports, either put them in a separate lint or not lint on them at all.
Policy feelings: Also, by policy, we shouldn't extend existing warn-by-default or higher lints with new things that add substantial new warnings. tmandry has a more full policy for us to review here.
(The second part of the meeting ended here.)
Self
ctor from outer item is referenced in inner nested item" rust#124187Link: https://github.com/rust-lang/rust/pull/124187
TC: CE nominates this for us:
This implements a warning
SELF_CONSTRUCTOR_FROM_OUTER_ITEM
when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned #117246 (comment).This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically never correct to do.
This also reverts part of #117246, since I believe this is the proper fix and we shouldn't need the helper functions (
opt_param_at
/opt_type_param
) any longer, since they shouldn't really ever be used in cases where we don't have this problem.
TC: What do we think?
elided_lifetimes_in_associated_constant
to deny" rust#124211Link: https://github.com/rust-lang/rust/pull/124211
TC: CE nominates this for us, noting that it's been 5 versions since this was last bumped. What do we think?
reserve-x18
target feature for aarch64" rust#124323Link: https://github.com/rust-lang/rust/pull/124323
The RfL folks are proposing an insta-stable reserve-x18
target feature for aarch64. There's an interesting argument here about why this does not affect the ABI in ways we've been worried about with target features recently:
it does not affect the ABI in a way where it is a problem to mix code with and without the feature. From the ABI spec:
X18 is the platform register and is reserved for the use of platform ABIs. This is an additional temporary register on platforms that don't assign a special meaning to it.
That is to say, the register is either already reserved (this is the case on Android targets), or it is a caller-saved temporary register (this is the case on aarch64-unknown-none). Changing a register from caller-saved temporary register to reserved is not breaking, so selectively enabling reserve-x18 on some compilation targets (or even on specific functions) cannot result in UB.
That said, removing the reserve-x18 target feature from a function can potentially trigger UB under some circumstances. This is because it is UB to link together -Zsanitizer=shadow-call-stack code with code where x18 is a temporary register. So enabling SCS in a binary requires that x18 is reserved globally. However, right now -Zsanitizer=shadow-call-stack can only be used on targets such as Android where x18 is never a temporary register, so this shouldn't be an issue for this PR.
The PR also notes some alternatives that were considered.
In the issue, Urgau noted this could be merged without our OK if this were changed to not be insta-stable. The author indicated that insta-stable is probably desirable here.
I brought this issue to the attention of RalfJ who has been working on ABI issues recently. He suggests that this may be more in line with the proposals for ABI variants, as that's more what this is than a target feature.
TC: What do we think?
Link: https://github.com/rust-lang/rust/issues/122301
TC: The8472 asks whether this code, which compiles today, can be relied upon:
const fn panic<T>() {
struct W<T>(T);
impl<T> W<T> {
const C: () = panic!();
}
W::<T>::C
}
struct Invoke<T, const N: usize>(T);
impl<T, const N: usize> Invoke<T, N> {
const C: () = match N {
0 => (),
// Not called for `N == 0`, so not monomorphized.
_ => panic::<T>(),
};
}
fn main() {
let _x = Invoke::<(), 0>::C;
}
The8472 notes that this is a useful property and that there are use cases for this in the compiler and the standard library, at least unless or until we adopt something like const if
:
https://github.com/rust-lang/rfcs/issues/3582
RalfJ has pointed out to The8472 that the current behavior might not be intentional and notes:
It's not opt-dependent, but it's also unclear how we want to resolve the opt-dependent issue. Some proposals involve also walking all items "mentioned" in a const. That would be in direct conflict with your goal here I think. To be clear I think that's a weakness of those proposals. But if that turns out to be the only viable strategy then we'll have to decide what we want more: using
const
tricks to control what gets monomorphized, or not having optimization-dependent errors.One crucial part of this construction is that everything involved is generic. If somewhere in the two "branches" you end up calling a monomorphic function, then that may have its constants evaluated even if it is in the "dead" branch – or it may not, it depends on which functions are deemed cross-crate-inlinable. That's basically what #122814 is about.
TC: The question to us is whether we want to guarantee this behavior. What do we think?
Link: https://github.com/rust-lang/rust/pull/124048
TC: We support accessing variadic external functions with, e.g.:
extern "C" {
fn foo(x: i32, ...);
fn with_name(format: *const u8, args: ...);
}
(See the reference.)
However, apparently we don't support variadic functions with no fixed parameters, e.g.:
extern "C" {
fn foo(...);
}
This was always legal in C but not useful and so not used in practice. Since C23, this is apparently useful and so people want this now.
TC: What do we think?
macro_metavar_expr_concat
" rust#124225Link: https://github.com/rust-lang/rust/issues/124225
TC: This is an experiment seeking a liaison. Does this spark joy for anyone?
min_exhaustive_patterns
" rust#122792Link: https://github.com/rust-lang/rust/pull/122792
TC: Nadri proposes that we stabilize min_exhaustive_patterns
:
With this feature, patterns of empty types are considered unreachable when matched by-value. This allows:
enum Void {}
fn foo() -> Result<u32, Void>;
fn main() {
let Ok(x) = foo();
// also
match foo() {
Ok(x) => ...,
}
}
This is a subset of the long-unstable
exhaustive_patterns
feature. That feature is blocked because omitting empty patterns is tricky when not matched by-value. This PR stabilizes the by-value case, which is not tricky.The not-by-value cases (behind references, pointers, and unions) stay as they are today, e.g.
enum Void {}
fn foo() -> Result<u32, &Void>;
fn main() {
let Ok(x) = foo(); // ERROR: missing `Err(_)`
}
The consequence on existing code is some extra "unreachable pattern" warnings. This is fully backwards-compatible.
TC: Further details are here:
https://github.com/rust-lang/rust/pull/122792#issue-2198466801
We discussed this in the 2024-03-27 meeting, and scottmcm proposed FCP merge.
Since then, it's been observed that there's a relationship between this and:
https://github.com/rust-lang/rust/pull/108993
We had delegated this issue to T-types on 2023-03-24. It had then blocked pending the stabilization of exhaustive patterns, as those were felt needed to land this:
https://github.com/rust-lang/rust/pull/108993#issuecomment-1540087895
It's now been observed that, if we're going to, we need to land #108993 before or simultaneously with stabilizing exhaustive patterns (completing the cycle…). This is due to code such as:
#![feature(min_exhaustive_patterns)]
#![allow(unused)]
fn infer<F: FnOnce() -> R, R>(_: Result<bool, R>) {}
enum Void {}
fn with_void() {
let mut x = Ok(true);
match x {
Ok(_) => {}
}
infer::<fn() -> Void, _>(x)
}
TC: What do we think?
Link: https://github.com/rust-lang/rust/issues/49804
TC: fmease nominates this for us:
Nominating Ralf's comment for T-lang discussion. Context for T-lang: There's currently active compiler dev going on to implement this feature (several merged and open PRs by multiple contributors). I don't want them to continue working on it if it gets thrown out in the end.
TC: That comment from RalfJ is:
Unresolved question: what should
derive
macros do here? This applies both to the built-in ones and user-defined ones. It seems like they all need major overhaul to support types like this. And it is pretty inevitable that people will ask forderive
to be supported on these types, even if the MVP does not support them.OTOH I assume many of them don't support unions to begin with, and these unnamed fields only really make sense when there are unions involved I think?
The RFC also explicitly lists anonymous types as a rejected alternative, and yet the implementation that recently began for this RFC does introduce anonymous ADTs to the compiler. Though maybe if it is impossible to write an expression of these types they are less problematic? That said I assume in the internal compiler IRs such expressions will exist – the unnamed fields are getting an internal name and field accesses are desugared to use those names.
It's that kind of issue that makes me think that adding a new form of unnamed types to Rust (on top of closures/coroutines) is a mistake. The RFC was accepted 6 years ago, our approach to language design and evolution changed since then. I think we need to ensure that this is even still something we want to do in this form.
TC: What do we think?
#[coverage]
" rust#84605Link: https://github.com/rust-lang/rust/issues/84605
TC: This is about stabilizing a #[coverage(off)]
attribute to exclude items from -Z instrument-coverage
.
Josh proposed FCP merge and nominated this for us.
There are two open questions about applying this automatically to nested functions and to inlined functions.
TC: What do we think?
extended_varargs_abi_support
" rust#116161Link: https://github.com/rust-lang/rust/pull/116161
TC: This stabilization was nominated for us, with pnkfelix commenting:
Just to add on to @cjgillot 's comment above: @wesleywiser and I could not remember earlier today whether T-lang wants to own FCP'ing changes like this that are restricted to extending the set of calling-conventions (i.e. the
conv
inextern "conv" fn foo(...)
), which is largely a detail about what platforms one is interoperating with, and not about changing the expressiveness of the Rust language as a whole in the abstract.(My own gut reaction is that T-compiler is a more natural owner for this than T-lang, but I wasn't certain and so it seems best to let the nomination stand and let the two teams duke it out.)
TC: What do we think about 1) this stabilization, and 2) whether we want to own this?
Link: https://github.com/rust-lang/rust/pull/120221
TC: CE handed this one to us, since it changes the contract of macro matchers.
Here's the code that does not work today that we would make work:
macro_rules! m {
($pat:pat) => {};
($stmt:stmt) => {};
}
macro_rules! m2 {
($stmt:stmt) => {
m! { $stmt }
//~^ ERROR expected pattern
};
}
m2! { let x = 1 }
This code does not work because we consider :stmt
to be a possible :pat
even though we then always reject it later in the process. By saying that :stmt
cannot be a :pat
, we make this code work.
We discussed this in the meeting on 2024-03-27:
CE: Right now the tokens that a macro matcher may begin with is a stable guarantee. We are relaxing the assumption that pattern matchers may begin with statement metavariables ($var whose type is stmt), because when we actually try to parse such a pattern, we are always guaranteed to fail. This only allows more code to compile, and would only break future code if we specifically wanted to begin patterns with statement metavariable.
scottmcm: I agree that it's weird to allow a
:stmt
in a pattern, so am happy to say we won't. Let's see what others think, since this conversation was in a sparsely-attended triage meeting:scottmcm: The other thing we explored was what it would take to make this actually work, since you can actually put an
:expr
into a pattern. But CE argued that we don't actually like that that works, it's just something we're stuck with because people used it before:literal
was available, which seems fair.
TC: What do we think?
Link: https://github.com/rust-lang/rust/pull/120706
TC: This is related to this MCP about a path toward async drop and scoped tasks:
https://github.com/rust-lang/compiler-team/issues/727
TC: petrochenkov gives some background:
So, what are the goals here:
- We want to have a possibility to add new auto traits that are added to all bound lists by default on the current edition. The examples of such traits could be
Leak
,Move
,SyncDrop
or something else, it doesn't matter much right now. The desired behavior is similar to the currentSized
trait. Such behavior is required for introducing!Leak
or!SyncDrop
types in a backward compatible way. (BothLeak
andSyncDrop
are likely necessary for properly supporting libraries for scoped async tasks and structured concurrency.)- It's not clear whether it can be done backward compatibly and without significant perf regressions, but that's exactly what we want to find out. Right now we encounter some cycle errors and exponential blow ups in the trait solver, but there's a chance that they are fixable with the new solver.
- Then we want to land the change into rustc under an option, so it becomes available in bootstrap compiler. Then we'll be able to do standard library experiments with the aforementioned traits without adding hundreds of
#[cfg(not(bootstrap))]
s.- Based on the experiments, we can come up with some scheme for the next edition, in which such bounds are added more conservatively.
- Relevant blog posts - https://without.boats/blog/changing-the-rules-of-rust/, https://without.boats/blog/follow-up-to-changing-the-rules-of-rust/ and https://without.boats/blog/generic-trait-methods-and-new-auto-traits/, https://without.boats/blog/the-scoped-task-trilemma/
- Larger compiler team MCP including this feature - MCP: Low level components for async drop compiler-team#727, it gives some more context
We discussed this in the async WG on 2024-03-25 and commented:
This is interesting work, but there's a lot to review here. We'd be particularly interested in seeing something in the way of a design document here, specifically e.g. with respect to when these bounds are added and when they are not, and how they interact with the
?
bounds. Seeing the algorithm spelled out in words and in theory would definitely help us understand this. The best place to put this may be in the rustc-dev-guide.
The question here is whether we want to charter this as an experiment.
#[expect]
some lints: Stabilize lint_reasons
(RFC 2383) " rust#120924Link: https://github.com/rust-lang/rust/pull/120924
TC: Since the last time this was proposed for stabilization, various unresolved questions have now been resolved, so this is being proposed again.
We're talking about this:
#![feature(lint_reasons)]
fn main() {
#[deny(unused_variables, reason = "unused variables, should be removed")]
let unused = "How much wood would a woodchuck chuck?";
}
error: unused variable: `unused`
--> src/main.rs:5:9
|
5 | let unused = "How much wood would a woodchuck chuck?";
| ^^^^^^ help: if this is intentional, prefix it with an underscore: `_unused`
|
= note: unused variables, should be removed
note: the lint level is defined here
--> src/main.rs:4:12
|
4 | #[deny(unused_variables, reason = "unused variables, should be removed")]
| ^^^^^^^^^^^^^^^^
And this:
#![feature(lint_reasons)]
fn main() {
#[expect(unused_variables, reason = "WIP, I'll use this value later")]
let message = "How much wood would a woodchuck chuck?";
#[expect(unused_variables, reason = "is this unused?")]
let answer = "about 700 pounds";
println!("A: {answer}")
}
warning: this lint expectation is unfulfilled
--> src/main.rs:4:14
|
6 | #[expect(unused_variables, reason = "is this unused?")]
| ^^^^^^^^^^^^^^^^
|
= note: `#[warn(unfulfilled_lint_expectations)]` on by default
= note: is this unused?
On 2024-03-15, tmandry proposed FCP merge, and nikomatsakis is also +1. This needs one more +1 to go into FCP. What do we think?
Link: https://github.com/rust-lang/rust/pull/121676
TC: This is related to this MCP about a path toward async drop and scoped tasks:
https://github.com/rust-lang/compiler-team/issues/727
TC: petrochenkov gives some background:
Summary:
- Initial support for auto traits with default bounds #120706 introduces a way to add new auto traits that are appended to all bound lists by default, similarly to existing
Sized
. Such traits may includeLeak
,SyncDrop
or similar, see Initial support for auto traits with default bounds #120706 (comment) for more detailed motivation.- To opt out from bounds added by default the
?Trait
syntax is used, but such "maybe" bounds are not supported in some contexts like supertrait lists anddyn Trait + ...
lists, becauseSized
is not added by default in those context.- This PR adds a feature for supporting
trait Trait1: ?Trait2
,dyn Trait1 + ?Trait2
and also multiple maybe bounds in the same list?Trait1 + ?Trait2
, because the new traits need to be added by default in those contexts too, and?Sized + ?Leak
may also make sense.- We need this to be available in bootstrap compiler, to make experiments on standard library without adding too many
#[cfg(not(bootstrap))]
s- Larger compiler team MCP including this feature - MCP: Low level components for async drop compiler-team#727, it gives some more context
TC: The question here is whether we want to charter this as an experiment.
Link: https://github.com/rust-lang/rust/pull/121965
TC: scottmcm filed this issue and explains:
The length limit on slices is clearly a safety invariant, and I'd like it to also be a validity invariant. With function parameter metadata making progress in LLVM, I'd really like to be able to use it when
&[_]
is passed as a scalar pair, in particular.The documentation for references is cagey about what exactly is a validity invariant, so for now just elaborate on the consequences of the existing safety rules on slices – the length restriction follows from the
size_of_val
restriction – as a way to help discourage people from trying to violate them.I also made the existing warning stronger, since I'm fairly sure it's already UB to violate at least the "references must be non-null" rule, rather than it just being that it "might be UB in the future".
Then joboet nominated this for us with:
Given that
slice::from_raw_parts
already states that "the total sizelen * mem::size_of::<T>()
of the slice must be no larger thanisize::MAX
" and that its behaviour is undefined otherwise, I'd say that this is entirely uncontroversial. Still, I'd appreciate some team sign-off on this, I think this concerns lang?
RalfJ thinks this should probably be a dual T-lang / T-opsem FCP.
TC: What do we think?
#![crate_name = EXPR]
semantically allows EXPR
to be a macro call but otherwise mostly ignores it" rust#122001Link: https://github.com/rust-lang/rust/issues/122001
TC: In previous stable versions of Rust, #![crate_name = EXPR]
worked. That is, within EXPR
we expanded and then used macro calls such as concat
.
However, due to:
https://github.com/rust-lang/rust/pull/117584
…we broke this, and then we shipped it in stable Rust v1.77.
Except, we only half broke it. It doesn't work, but neither is it a hard error. It just quietly ignores the result.
We discussed this in the meeting on 2024-03-27 and agreed this was the worst of all worlds, and so we should at a minimum break it completely, and then we could always later decide to relax the hard error and make it work again by reverting #117584. On that basis, scottmcm proposed FCP merge.
TC: What do we think?
assert!
expression is bool
" rust#122661Link: https://github.com/rust-lang/rust/pull/122661
TC: estebank describes this issue for us:
In the desugaring of
assert!
in 2024 edition, assign the condition expression to abool
biding in order to provide better type errors when passed the wrong thing.The span will point only at the expression, and not the whole
assert!
invocation.
error[E0308]: mismatched types
--> $DIR/issue-14091.rs:2:13
|
LL | assert!(1,1);
| ^ expected `bool`, found integer
We no longer mention the expression needing to implement the
Not
trait.
error[E0308]: mismatched types
--> $DIR/issue-14091-2.rs:15:13
|
LL | assert!(x, x);
| ^ expected `bool`, found `BytePos`
In <=2021 edition, we still accept any type that implements
Not<Output = bool>
.
TC: And pnkfelix nominates this for us:
At the very least, we might need to tie such a change to an edition.
I am not certain whether this decision would be a T-lang matter or a T-libs-api one. I'll nominate for T-lang for now.
(Namely: The question is whether we can start enforcing a rule that the first expression to
assert!
must be of bool type, which is how the macro is documented, but its current behavior is a little bit more general, as demonstrated in my prior comment)…
There is a design space here. E.g. one set of options is:
- (stable Rust behavior): in all editions, support arbitrary
impl Not<Output=bool>
for first parameter toassert!
;- in edition >= 2024, support just
Deref<Target=bool>
for first parameter toassert!
(e.g. by expanding tolet x: &bool = &$expr;
), or- (this PR): in edition >= 2024, support just
bool
for first parameter toassert!
.(And then there's variations thereof about how to handle editions < 2024, but that's a separate debate IMO.)
TC: What do we think?
match
is too complex" rust#122685Link: https://github.com/rust-lang/rust/pull/122685
TC: Nadri nominates this for us and describes the situation:
Dear T-lang, this PR adds a warning that cannot be silenced, triggered when a match takes a really long time to analyze (in the order of seconds). This is to help users figure out what's taking so long and fix it.
We could make the limit configurable or the warning
allow
able. I argue that's not necessary because crater showed zero regressions with the current limit, and it's be pretty easy in general to split up amatch
into smallermatch
es to avoid blowup.We're still figuring out the exact limit, but does the team approve in principle?
(As an aside, awhile back someone showed how to lower SAT to exhaustiveness checking with match
. Probably that would hit this limit.)
TC: What do we think?
Link: https://github.com/rust-lang/rust/issues/122759
TC: In the call on 2024-03-13, we discussed this issue raised by tmandry:
"Fallout from expansion of redundant import checking"
https://github.com/rust-lang/rust/issues/121708
During the call, the thoughts expressed included:
TC: tmandry volunteered to draft a policy proposal. He's now written up this proposal in this issue.
Background
When a lint is expanded to include many new cases, it adds significant complexity to the rollout of a toolchain to large codebases. Maintainers of these codebases are stuck with the choice of
- Disabling the existing lint while the toolchain is updated and new cases are fixed
- Fixing cases manually and updating the toolchain immediately
Both of these come with the problem of racing with other developers in a codebase who may land new code which triggers the expanded lint in a new compiler, but does not trigger the lint in an old compiler.
While it would be nice to solve this "raciness" once and for all, there are other considerations at play. Instead, we propose to support these users by either providing them with a new lint name to temporarily opt out of OR a machine-applicable fix which eases the pain of any races which might occur.
Note that this requirement only applies to significant lint expansions as measured by crater.
Policy
When an existing lint is expanded to include many new cases, we must provide either:
- A new lint name under the existing group, so that users may opt out of the expansion at least temporarily, or
- A MachineApplicable fix for the lint.
Exceptions to this policy may be made via Language Team FCP.
Here, we define "many new cases" as impacting more than 5% of the top-1000 crates on crates.io. This can be measured by counting the number of regressions from a crater run like the one below.
A crater run is not required before landing for every lint expansion. Reviewers should use their best judgment to decide if one is required. However, if a lint expansion lands that violates this requirement, or is strongly suspected to violate this requirement based on other impact, it should be reverted.
Crater command
To measure the impact of a lint as defined by this policy, you can use the following crater command:
@craterbot run name=<name> start=master#<hash1>+rustflags=-D<lint_name> end=master#<hash2>+rustflags=-D<lint_name> crates=top-1000 mode=check-only p=1
See the crater docs for more information.
TC: What do we think?
Link: https://github.com/rust-lang/rfcs/pull/3098
TC: We've at various times discussed that we had earlier decided that if we wanted to use a new keyword within an edition, we would write it as k#keyword
, and for that reason, we prefer to not speculatively reserve keywords ahead of an edition (except, perhaps, when it's clear we plan to use it in the near future).
TC: Somewhat amusingly, however, we never in fact accepted that RFC. Back in 2021, we accepted scottmcm's proposal to cancel:
We discussed this RFC again in the lang team triage meeting today.
For the short-term goal of the reservation for the edition, we'll be moving forward on #3101 instead. As such, we wanted to leave more time for conversations about this one, and maybe use crater results from 3101 to make design changes,
@rfcbot cancel
Instead we accepted RFC 3101 that reserved ident#foo
, ident"foo"
, ident'f'
, and ident#123
starting in the 2023 edition.
Reading through the history, here's what I see:
k#keyword
, but it's another to actually do it in the face of certain criticism about that being e.g. unergonomic. Would we follow through?TC: What do we think?
Link: https://github.com/rust-lang/rust/pull/117329
TC: RalfJ nominates this for us:
Nominating for t-lang discussion. This implements the t-opsem consensus from rust-lang/opsem-team#10, rust-lang/unsafe-code-guidelines#472 to generally allow zero-sized accesses on all pointers. Also see the tracking issue.
- Zero-sized reads and writes are allowed on all sufficiently aligned pointers, including the null pointer
- Inbounds-offset-by-zero is allowed on all pointers, including the null pointer
offset_from
on two pointers is always allowed when they have the same address (but see the caveat below)This means the following function is safe to be called on any pointer:
fn test_ptr(ptr: *mut ()) { unsafe {
// Reads and writes.
let mut val = *ptr;
*ptr = val;
ptr.read();
ptr.write(());
// Memory access intrinsics.
// - memcpy (1st and 2nd argument)
ptr.copy_from_nonoverlapping(&(), 1);
ptr.copy_to_nonoverlapping(&mut val, 1);
// - memmove (1st and 2nd argument)
ptr.copy_from(&(), 1);
ptr.copy_to(&mut val, 1);
// - memset
ptr.write_bytes(0u8, 1);
// Offset.
let _ = ptr.offset(0);
let _ = ptr.offset(1); // this is still 0 bytes
// Distance.
let ptr = ptr.cast::<i32>();
ptr.offset_from(ptr);
} }
Some specific concerns warrant closer scrutiny.
LLVM 16
We currently still support LLVM 16, which does not yet have the patches that make
getelementptr inbounds
always well-defined for offset 0. The function above thus generates LLVM IR with UB. No known miscompilations arise from that, and my attempt at just removing theinbounds
annotation on old versions of LLVM failed (I got segfaults, and Nikic suggested that keeping these attribute around is actually less risky than removing them). If we want to avoid this, we have to wait until support for LLVM 16 can be dropped (which apparently is in May).Null pointers
t-opsem decided to allow zero-sized reads and writes on null pointers. This is mostly for consistency: we definitely want to allow zero-sized offsets on null pointers (
ptr::null::<T>().offset(0)
), since this is allowed in C++ (and a proposal is being made to allow it in C) and there's no reason for us to have more UB than C++ here. But if we allow this, and therefore consider the null pointer to have a zero-sized region of "inbounds" memory, then it would be inconsistent to not allow reading from / writing to that region.
offset_from
This operation is somewhat special as it takes two pointers. We do want
test_ptr
above to be defined on all pointers, sooffset_from
between two identical pointers without provenance must be allowed. But we also want to achieve this property called "provenance monotonicity", whereby adding arbitrary provenance to any no-provenance pointer must never make the program UB.1 From these two it follows that callingoffset_from
with two pointers with the same address but arbitrary different provenance must be allowed. This does have some minor downsides. So my proposal (and this goes beyond what t-opsem agreed on) is to define theptr_offset_from
intrinsic to satisfy provenance monotonicity, but to document the user-facingptr.offset_from(...)
as requiring either two pointers without provenance or two pointers with provenance for the same allocation – therefore, making the case of provenance mismatch library UB, but not language UB.Footnotes
- This property should hopefully make some intuitive sense, and it is also crucial to justify optimizations that make the program have more provenance than before – such as optimizing away provenance-stripping operations. Specifically,
*ptr = *ptr
whereptr: *mut usize
is likely going to be a provenance-stripping operation, and so optimizing away this redundant assignment requires provenance monotonicity. ↩
TC: What do we think?
Link: https://github.com/rust-lang/rust/pull/120193
TC: Apparently our unstable likely
and unlikely
intrinsics don't work. There's a proposal to do some work on fixing that and stabilizing a solution here. The nominated question is whether we want to charter this as an experiment.
Link: https://github.com/rust-lang/rfcs/pull/3514
TC: In addition to documenting the current behavior carefully, this RFC (per RalfJ)…
says we should allow float operations in
const fn
, which is currently not stable. This is a somewhat profound decision since it is the first non-deterministic operation we stably allow inconst fn
. (We already allow those operations inconst
/static
initializers.)
TC: What do we think? tmandry proposed this for FCP merge back in October 2023.
Link: https://github.com/rust-lang/rust/issues/116907
TC: nnethercote has implemented most of RFC 3349 ("Mixed UTF-8 literals") and, based on implementation experience, argues that the remainder of the RFC should not be implemented:
I have a partial implementation of this RFC working locally (EDIT: now at #120286). The RFC proposes five changes to literal syntax. I think three of them are good, and two of them aren't necessary.
TC: What do we think?
i128
/u128
from the improper_ctypes
lint" lang-team#255Link: https://github.com/rust-lang/lang-team/issues/255
TC: Trevor Gross describes the situation:
For a while, Rust's 128-bit integer types have been incompatible with those from C. The original issue is here rust-lang/rust#54341, with some more concise background information at the MCP here rust-lang/compiler-team#683
The current Beta of 1.77 will have rust-lang/rust#116672, which manually sets the alignment of
i128
to make it ABI-compliant with any version of LLVM (clang
does something similar now). 1.78 will have LLVM18 as the vendored version which fixes the source of this error.Proposal: now that we are ABI-compliant, do not raise
improper_ctypes
on our 128-bit integers. I did some testing with abi-cafe and a more isolated https://github.com/tgross35/quick-abi-check during the time https://reviews.llvm.org/D86310 was being worked on, and verified everything lines up. (It would be great to have some fork of abi-cafe in tree, but that is a separate discussion.)@joshtriplett mentioned that changing this lint needs a lang FCP https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/LLVM.20alignment.20of.20i128/near/398422037. cc @maurer
Reference change from when I was testing rust-lang/rust@c742908
TC: Josh nominates this for our discussion. What do we think?
is
operator for pattern-matching and binding" rfcs#3573Link: https://github.com/rust-lang/rfcs/pull/3573
TC: Josh proposes for us that we should accept:
if an_option is Some(x) && x > 3 {
println!("{x}");
}
And:
func(x is Some(y) && y > 3);
TC: The main topic discussed in the issue thread so far has been the degree to which Rust should have "two ways to do things". Probably the more interesting issue is how the binding and drop scopes for this should work.
TC: In the 2024-02-21 meeting (with limited attendance), we discussed how we should prioritize stabilizing let chains, and tmandry suggested we may want to allow those to settle first.
TC: What do we think, as a gut check?
Link: https://github.com/rust-lang/rfcs/pull/3556
TC: This seems to be about making the following work:
// kind is optional if it's been specified elsewhere, e.g. via the `-l` flag to rustc
#[link(name="ext", kind="static")]
extern {
#[no_mangle]
pub fn foo();
#[no_mangle]
pub static bar: std::ffi::c_int;
}
There are apparently use cases for this.
What's interesting is that apparently it already does, but we issue a warning that is wrong:
warning: `#[no_mangle]` has no effect on a foreign function
--> src/lib.rs:21:5
|
21 | #[no_mangle]
| ^^^^^^^^^^^^ help: remove this attribute
22 | pub fn foo_rfc3556_pub_with_no_mangle();
| ---------------------------------------- foreign function
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: symbol names in extern blocks are not mangled
TC: One of the author's asks of us is that we don't make this into a hard error (e.g. with the new edition).
TC: What do we think?
Link: https://github.com/rust-lang/rust/pull/118939
TC: The idea here seems to be to improve some diagnostics around macro_rules
, but this seems to be done by way of reserving the macro_rules
token more widely, which is a breaking change. Petrochenkov has objected to it on that basis, given that reserving macro_rules
minimally has been the intention since we hope it will one day disappear in favor of macro
. What do we think?
clippy::invalid_null_ptr_usage
lint" rust#119220Link: https://github.com/rust-lang/rust/pull/119220
TC: Urgau proposes this for us:
This PR aims at uplifting the
clippy::invalid_null_ptr_usage
lint into rustc, this is similar to theclippy::invalid_utf8_in_unchecked
uplift a few months ago, in the sense that those two lints lint on invalid parameter(s), here a null pointer where it is unexpected and UB to pass one.
invalid_null_ptr_usages
(deny-by-default)
The
invalid_null_ptr_usages
lint checks for invalid usage of null pointers.Example
// Undefined behavior
unsafe { std::slice::from_raw_parts(ptr::null(), 0); }
// Not Undefined behavior
unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), 0); }
Produces:
error: calling this function with a null pointer is undefined behavior, even if the result of the function is unused, consider using a dangling pointer instead
--> $DIR/invalid_null_ptr_usages.rs:14:23
|
LL | let _: &[usize] = std::slice::from_raw_parts(ptr::null(), 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^^^^
| |
| help: use a dangling pointer instead: `core::ptr::NonNull::dangling().as_ptr()`
Explanation
Calling methods who's safety invariants requires non-null pointer with a null pointer is undefined behavior.
The lint use a list of functions to know which functions and arguments to checks, this could be improved in the future with a rustc attribute, or maybe even with a
#[diagnostic]
attribute.
TC: What do we think?
Link: https://github.com/rust-lang/rust/pull/122412
TC: Waffle nominates this breaking change for us:
This changes
expr?
's desugaring like so (simplified, see code for more info):// old match expr { Ok(val) => val, Err(err) => return Err(err), } // new match expr { Ok(val) => val, Err(err) => core::convert::absurd(return Err(err)), } // core::convert pub const fn absurd<T>(x: !) -> T { x }
This prevents
!
from thereturn
from skewing inference:// previously: ok (never type spontaneous decay skews inference, `T = ()`) // with this pr: can't infer the type for `T` Err(())?;
We discussed this on 2024-03-20. On the one hand, people were hesitant to block incremental progress, but on the other, people were hesitant to add a special case if we could address a more general case. There was, I would say, appetite for taking a bigger bite here, but people were uncertain if there were any bigger bites that were feasible other than those discussed to support the never type generally, such as disabling fallback to ()
.
In terms of next steps, we wanted to see an answer about the pros and cons of doing this for return
generally, which @WaffleLapkin has now answered:
it made me wonder whether it would be feasible to change return in general to be a free type variable instead of
!
?@scottmcm I'm not sure. I don't think it's unfeasible, but it sure is harder than this.
The issues are:
- Need to add fallback for those type variables too, so that
return;
works{ return; }
(which is currently!
even though there is;
) needs to be special cased in a different way- Will break strictly more things
I'm not sure if this is a good idea or not. It's kinda weird.
…and we wanted to see the results of the crater run that we know that @WaffleLapkin is working to make happen.
When taking this back up, in addition to those details, we wanted to specifically consider how this incremental step may be addressing known footguns with unsafe code such as that in:
https://github.com/rust-lang/rust/issues/51125
TC: What do we think?
Link: https://github.com/rust-lang/rust/issues/123231
TC: RalfJ nominates this for us. Consider this code:
#![feature(c_unwind)]
struct Noise;
impl Drop for Noise {
fn drop(&mut self) {
eprintln!("Noisy Drop");
}
}
extern "C" fn test() {
let _val = Noise;
panic!("heyho");
}
fn main() {
test();
}
It doesn't print anything. Should it?
const {}
blocks, and const { assert!(...) }
" lang-team#251Link: https://github.com/rust-lang/lang-team/issues/251
TC: This issue was raised due to discussion in a T-libs-api call. Josh gives the context:
In discussion of rust-lang/libs-team#325 (a proposal for a compile-time assert macro), the idea came up to allow
const {}
blocks at item level, and then have people useconst { assert!(...) }
.@rust-lang/libs-api would like some guidance from @rust-lang/lang about whether lang is open to toplevel
const { ... }
blocks like this, which would influence whether we want to add a compile-time assert macro, as well as what we want to call it (e.g.static_assert!
vsconst_assert!
vs some other name).Filing this issue to discuss in a lang meeting. This issue is not seeking any hard commitment to add such a construct, just doing a temperature check.
CAD97 noted:
To ensure that it's noted: if both item and expression
const
blocks are valid in the same position (i.e. in statement position), a rule to disambiguate would be needed (like for statement versus expressionif
-else
). IMO it would be quite unfortunate for item-levelconst
blocks to be evaluated pre-mono if that sameconst
block but statement-level would be evaluated post-mono.Additionally: since
const { assert!(...) }
is post-mono (due to using the generic context), it's potentially desirable to push people towards usingconst _: () = assert!(...);
(which is pre-mono) whenever possible (not capturing generics).
TC: What do we think?
Link: https://github.com/rust-lang/rfcs/pull/3325
TC: On 2024-03-25, Josh resolved the concern he had raised regarding the syntax, so this is now entering FCP.
Unless someone raises a new concern on syntax, the syntax will be:
#[unsafe(attr)]
("unsafe parens")
There was a poll that showed strong support for this syntax:
https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Unsafe.20attribute.20syntax
Link: https://github.com/rust-lang/rust/pull/118833
TC: In the 2024-01-03 call, we developed a tentative consensus to lint against direct function pointer comparison and to push people toward using ptr::fn_addr_eq
. We decided to ask T-libs-api to add this. There's now an open proposal for that here:
https://github.com/rust-lang/libs-team/issues/323
One question that has come up is whether we would expect this to work like ptr::addr_eq
and have separate generic parameters, e.g.:
/// Compares the *addresses* of the two pointers for equality,
/// ignoring any metadata in fat pointers.
///
/// If the arguments are thin pointers of the same type,
/// then this is the same as [`eq`].
pub fn addr_eq<T: ?Sized, U: ?Sized>(p: *const T, q: *const U) -> bool { .. }
Or whether we would prefer that fn_addr_eq
enforced type equality of the function pointers. Since we're the ones asking for this, we probably want to develop a consensus here. We discussed this in the call on 2024-01-10, then we opened a Zulip thread:
TC: On this subject, scottmcm raised this point, with which pnkfelix seemed to concur:
I do feel like if I saw code that had
fn1.addr() == fn2.addr()
(ifFnPtr
were stabilized), I'd write a comment saying "isn't that whatfn_addr_eq
is for?"If the answer ends up being "no, actually, because I have different types", that feels unfortunate even if it's rare.
(Like how
addr_eq(a, b)
is nice even if with strict provenance I could writea.addr() == b.addr()
anyway.)
TC: scottmcm also asserted confidence that allowing mixed-type pointer comparisons is correct for ptr::addr_eq
since comparing the addresses of *const T
, *const [T; N]
, and *const [T]
are all reasonable. I pointed out that, if that's reasonable, then ptr::fn_addr_eq
is the higher-ranked version of that, since for the same use cases, it could be reasonable to compare function pointers that return those three different things or accept them as arguments.
TC: Adding to that, scottmcm noted that comparing addresses despite lifetime differences is also compelling, e.g. comparing fn(Box<T>) -> &'static mut T
with for<'a> fn(Box<T>) -> &'a mut T
.
TC: Other alternatives we considered were not stabilizing ptr::fn_addr_eq
at all and instead stabilizing FnPtr
so people could write ptr::addr_eq(fn1.addr(), fn2.addr())
, or expecting that people would write instead fn1 as *const () == fn2 as *const ()
.
TC: Recently CAD97 raised an interesting alternative:
From the precedent of
ptr::eq
andptr::addr_eq
, I'd expect a "ptr::fn_eq
" to have one generic type and a "ptr::fn_addr_eq
" to have two. Even ifptr::fn_eq
's implementation is just an address comparison, it still serves as a documentation point to call out the potential pitfalls with comparing function pointers.
TC: What do we think?
TC: Separately, on the 2024-01-10 call, we discussed some interest use cases for function pointer comparison, especially when it's indirected through PartialEq
. We had earlier said we didn't want to lint when such comparisons were indirected through generics, but we did address the non-generic case of simply composing such comparisons.
One example of how this is used is in the standard library, in Waker::will_wake
:
https://doc.rust-lang.org/core/task/struct.Waker.html#method.will_wake
It's comparing multiple function pointers via a #[derive(PartialEq)]
on the RawWakerVTable
.
We decided on 2024-01-01 that this case was interesting and we wanted to think about it further. We opened a discussion thread about this:
Since then, another interesting use case in the standard library was raised, in the formatting machinery:
https://doc.rust-lang.org/src/core/fmt/rt.rs.html
What do we think about these, and would we lint on derived PartialEq
cases like these or no?
Link: https://github.com/rust-lang/rust/pull/121364
TC: The proposal is to lint against:
-2.pow(2); // Equals -4.
These would instead be written:
-(2.pow(2)); // Equals -4.
TC: This is a subset of:
https://github.com/rust-lang/rust/pull/117161
…which is also nominated. Whereas the #117161 proposal is to lint on both binary op and unary op cases, this proposal is to lint only on unary op cases. The proposal for this subset came out a discussion with scottmcm.
TC: What do we think?
clippy::precedence
lint" rust#117161Link: https://github.com/rust-lang/rust/pull/117161
TC: The proposal is to lint against:
-2.pow(2); // Equals -4.
1 << 2 + 3; // Equals 32.
These would instead be written:
-(2.pow(2)); // Equals -4.
1 << (2 + 3); // Equals 32.
Prompts for discussion:
rustc
?!
, *
, &
)?Link: https://github.com/rust-lang/rust/issues/62569
TC: Prior to main()
being executed, the Rust startup code makes a syscall to change the handling of SIGPIPE
. Many believe that this is wrong thing for a low-level language like Rust to do, because 1) it makes it impossible to recover what the original value was, and 2) means things like seccomp
filters must be adjusted for this.
It's also just, in a practical sense, wrong for most CLI applications.
This seems to have been added back when Rust had green threads and then forgotten about. But it's been an ongoing footgun.
Making a celebrity appearance, Rich Felker, the author of MUSL libc, notes:
As long as Rust is changing signal dispositions inside init code in a way that the application cannot suppress or undo, it is fundamentally unusable to implement standard unix utilities that run child processes or anything that needs to preserve the signal dispositions it was invoked with and pass them on to children. Changing inheritable process state behind the application's back is just unbelievably bad behavior and does not belong in a language runtime for a serious language…
As an example, if you implement
find
in Rust, the-exec
option will invoke its commands withSIGPIPE
set toSIG_IGN
, so that they will not properly terminate on broken pipe. But if you just made it setSIGPIPE
toSIG_DFL
before invoking the commands, now it would be broken in the case where the invoking user intentionally setSIGPIPE
toSIG_IGN
so that the commands would not die on broken pipe.
There was discussion in 2019 about fixing this over an edition, but nothing came of it.
Are we interested in fixing it over this one?
Strawman (horrible) proposal: We could stop making this pre-main syscall in Rust 2024 and have cargo fix
insert this syscall at the start of every main
function.
(In partial defense of the strawman, it gets us directly to the arguably best end result while having an automatic semantics-preserving edition migration and it avoids the concerns about lang/libs coupling that Mara raised. The edition migration could add a comment above this inserted code telling people under what circumstances they should either keep or delete the added line.)
Link: https://github.com/rust-lang/rust/issues/116557
TC: nikomatsakis nominated this:
We had some discussion about types/lang team interaction. We concluded a few things:
- Pinging the team like @rust-lang/lang is not an effective way to get attention. Nomination is the only official way to get attention.
- It's ok to nominate things in an "advisory" capacity but not block (e.g., landing a PR), particularly as most any action can ultimately be reversed. But right now, triagebot doesn't track closed issues, so that's a bit risky.
Action items:
- We should fix triagebot to track closed issues.
TC: What do we think?
PartialOrd
and Ord
for Discriminant
" rust#106418Link: https://github.com/rust-lang/rust/pull/106418
TC: We discussed this last in the meeting on 2024-03-13. scottmcm has now raised on concern on the issue and is planning to make a counter-proposal:
I remain concerned about exposing this with no opt-out on an unrestricted generic type @rfcbot concern overly-broad
I'm committing to making an alternative proposal because I shouldn't block without one. Please hold my feet to the fire if that's no up in a week.
Basically, I have an idea for how we might be able to do this, from #106418 (comment)
- Expose the variant ordering privately, only accessible by the type owner/module.
Solution 2. is obviously more desirable, but AFAIK Rust can't do that and there is no proposal to add a feature like that.
https://github.com/rust-lang/rust/pull/106418#issuecomment-1994833151
Link: https://github.com/rust-lang/rust/issues/121708
TC: We discussed this in the meeting on 2024-03-13. The feelings expressed included:
TC: tmandry volunteered to draft a policy proposal.
None.
Link: https://github.com/rust-lang/lang-team/pull/236
Link: https://github.com/rust-lang/lang-team/pull/237
Link: https://github.com/rust-lang/lang-team/pull/258
Link: https://github.com/rust-lang/rfcs/pull/3519
Link: https://github.com/rust-lang/rfcs/pull/3606
Link: https://github.com/rust-lang/rfcs/pull/3593
Link: https://github.com/rust-lang/rfcs/pull/3325
S-waiting-on-team
REDUNDANT_IMPORTS
lint for new redundant import detection" rust#123813Link: https://github.com/rust-lang/rust/pull/123813
Link: https://github.com/rust-lang/rust/pull/124048
min_exhaustive_patterns
" rust#122792Link: https://github.com/rust-lang/rust/pull/122792
Link: https://github.com/rust-lang/rust/pull/120221
count
, ignore
, index
, and length
in Rust 1.80" rust#122808Link: https://github.com/rust-lang/rust/pull/122808
Link: https://github.com/rust-lang/rust/pull/117329
Link: https://github.com/rust-lang/rust/pull/118939
Link: https://github.com/rust-lang/rust/pull/116675
Link: https://github.com/rust-lang/rust/pull/116863
AsyncIterator
back to Stream
, introduce an AFIT-based AsyncIterator
trait" rust#119550Link: https://github.com/rust-lang/rust/pull/119550
#[deny]
inside #[forbid]
as a no-op with a warning" rust#121560Link: https://github.com/rust-lang/rust/pull/121560
Check your boxes!
Link: https://github.com/rust-lang/rust/pull/123590
min_exhaustive_patterns
" rust#122792Link: https://github.com/rust-lang/rust/pull/122792
#[coverage]
" rust#84605Link: https://github.com/rust-lang/rust/issues/84605
Link: https://github.com/rust-lang/rust/pull/120221
#![crate_name = EXPR]
semantically allows EXPR
to be a macro call but otherwise mostly ignores it" rust#122001Link: https://github.com/rust-lang/rust/issues/122001
count
, ignore
, index
, and length
in Rust 1.80" rust#122808Link: https://github.com/rust-lang/rust/pull/122808
Link: https://github.com/rust-lang/rfcs/pull/3514
PartialOrd
and Ord
for Discriminant
" rust#106418Link: https://github.com/rust-lang/rust/pull/106418
Link: https://github.com/rust-lang/rfcs/pull/2375
Link: https://github.com/rust-lang/rfcs/pull/3288
Link: https://github.com/rust-lang/rfcs/pull/3379
anonymous_lifetime_in_impl_trait
" rust#107378Link: https://github.com/rust-lang/rust/pull/107378
const_cstr_from_ptr
" rust#113219Link: https://github.com/rust-lang/rust/issues/113219
Link: https://github.com/rust-lang/rust/pull/116675
Link: https://github.com/rust-lang/rust/pull/117468
Link: https://github.com/rust-lang/rust/pull/120700
#[deny]
inside #[forbid]
as a no-op with a warning" rust#121560Link: https://github.com/rust-lang/rust/pull/121560
Link: https://github.com/rust-lang/rust/issues/121608
Link: https://github.com/rust-lang/rfcs/pull/3484
IntoIterator
for Box<[T]>
+ edition 2024-specific lints" rust#124097Link: https://github.com/rust-lang/rust/pull/124097
Link: https://github.com/rust-lang/rfcs/pull/3550
#[expect]
some lints: Stabilize lint_reasons
(RFC 2383) " rust#120924Link: https://github.com/rust-lang/rust/pull/120924
Link: https://github.com/rust-lang/rfcs/pull/3336
Link: https://github.com/rust-lang/rfcs/pull/3617
None.