Created
November 1, 2023 15:08
-
-
Save pnkfelix/0b72a18f73ef4b346bb6b05b292772ee to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
--- | |
title: Triage meeting 2023-11-01 | |
tags: T-lang, triage-meeting, minutes | |
date: 2023-11-01 | |
discussion: https://rust-lang.zulipchat.com/#narrow/stream/410673-t-lang.2Fmeetings/topic/Triage.20meeting.202023-11-01 | |
url: https://hackmd.io/ZTUXE9EES9eNDX1GaLpoQw | |
--- | |
# T-lang meeting agenda | |
- Meeting date: 2023-11-01 | |
## Attendance | |
- People: TC, Urgau, waffle, pnkfelix, Josh, eholk, nikomatsakis, scottmcm | |
## Meeting roles | |
- Minutes, driver: | |
## Scheduled meetings | |
- 2023-11-01: Planning meeting | |
Edit the schedule here: https://github.com/orgs/rust-lang/projects/31/views/7. | |
## Announcements or custom items | |
(Meeting attendees, feel free to add items here!) | |
## Nominated RFCs, PRs, and issues | |
### "RFC: Syntax for embedding cargo-script manifests" rfcs#3503 | |
**Link:** https://github.com/rust-lang/rfcs/pull/3503 | |
TC: The following syntax is being proposed for addition to Rust: | |
````rust | |
#!/usr/bin/env cargo | |
```cargo | |
[dependencies] | |
clap = { version = "4.2", features = ["derive"] } | |
``` | |
use clap::Parser; | |
```` | |
TC: tmandry commented: | |
> With my lang hat on, I don't see a reason we should RFC a feature that only allows `cargo` front matter, without specifying a path to generalizing it to other tooling. If we want to be conservative in what we stabilize, let's approach that in the stabilization rather than in the RFC. | |
> | |
> At the language level we should acknowledge that not all projects get to use cargo, and the generalization here seems trivial to do in the RFC. Note that I'm fine with the RFC being conservative in other ways (only allowing one, right after the shebang, etc). | |
> | |
> Taking off my lang hat now – using `cargo` sure makes it awkward to extend to embedding a second front matter with Cargo.lock. It'd be nice if the RFC addressed that. (Maybe just say "we can use `cargo-lock` for consistency with `cargo`"?) | |
TC: +1 from me there. | |
TC: To perhaps prompt discussion, here are some thoughts I earlier wrote down when reviewing this RFC: | |
- The code string literals RFC had also proposed to use Markdown code block syntax. In the discussion surrounding that, the feeling was that may be a bridge too far. Would we feel differently here? | |
- Even if cargo never wanted to support `Cargo.lock` or other files, it's clear that other build tools may want to support these. Go's similar feature, `gorun`, has this same problem and therefore spells out the name of each file in the syntax. Are we sure we don't want to do that? | |
- The key T-lang concern here is specifying "what's allowed in a .rs file?" This has wider ecosystem implications on editors, syntax highlighters, rust-analyzer, etc. To what degree are we willing to break existing such tools with new syntax? And to what degree does this influence whether we should adopt a more general solution than just what works for Cargo's planned feature? | |
- Perhaps we should consider a new kind of comment for frontmatter in Rust 2024 that preserves the symmetry with existing comment types? E.g. we could use `//+` and `/*+ */`. This would have the advantage of not breaking existing parsers as it would be a legal comment in the old edition. | |
- There's overlap between this and the code string literals RFC. Both want to embed some structured non-Rust file in a Rust file. Both are hoping for eventual support from editors and other tools. Is there perhaps a way to build one on the other? E.g., perhaps we could add a `set_build_file!("Cargo.toml", ..)` macro where the second argument is expected (by convention) to be spelled with a code string literal. We could lint or error on "complicated" uses of that macro that may not be understood by tools. | |
- Some of the examples in the RFC don't specify the edition. The related RFC 3502 suggests these would be interpreted as refering to whatever the current edition is, and that breakage on `rustc` upgrades may be expected. Is this acceptable to T-lang? | |
TC: What thoughts do people have about this? | |
### "Stabilize Wasm target features that are in phase 4 and 5" rust#117457 | |
**Link:** https://github.com/rust-lang/rust/pull/117457 | |
TC: PR proposes: | |
> This stabilizes the Wasm target features that are known to be working and in [phase 4 and 5](https://github.com/WebAssembly/proposals/tree/04fa8c810e1dc99ab399e41052a6e427ee988180). | |
> | |
> Feature stabilized: | |
> | |
> * [Non-trapping float-to-int conversions](https://github.com/WebAssembly/nontrapping-float-to-int-conversions) | |
> * [Import/Export of Mutable Globals](https://github.com/WebAssembly/mutable-global) | |
> * [Sign-extension operators](https://github.com/WebAssembly/sign-extension-ops) | |
> * [Bulk memory operations](https://github.com/WebAssembly/bulk-memory-operations) | |
> * [Extended Constant Expressions](https://github.com/WebAssembly/extended-const) | |
> | |
> Features not stabilized: | |
> | |
> * [Multi-value](https://github.com/WebAssembly/multi-value): requires rebuilding `std` [Multi value Wasm compilation #73755](https://github.com/rust-lang/rust/issues/73755). | |
> * [Reference Types](https://github.com/WebAssembly/reference-types): no point stabilizing without [Support for WebAssembly externref in non-web environment #103516](https://github.com/rust-lang/rust/issues/103516). | |
> * [Threads](https://github.com/webassembly/threads): requires rebuilding `std` [Tracking issue for WebAssembly atomics #77839](https://github.com/rust-lang/rust/issues/77839). | |
> * [Relaxed SIMD](https://github.com/WebAssembly/relaxed-simd): separate PR [Stabilize Wasm relaxed SIMD #117468](https://github.com/rust-lang/rust/pull/117468). | |
> | |
> See [#117457 (comment)](https://github.com/rust-lang/rust/pull/117457#issuecomment-1787648070) for more context. | |
> | |
> Documentation: [rust-lang/reference#1420](https://github.com/rust-lang/reference/pull/1420) Tracking issue: #44839 | |
TC: alexcrichton helpfully filled in some background: | |
> AFAIK a stabilization policy is not documented anywhere for WebAssembly target features, but in my opinion this is a good PR to land. | |
> | |
> As a bit of background on this in case anyone's curious: WebAssembly target features in LLVM relate to [WebAssembly proposals](https://github.com/WebAssembly/proposals/) and are frequently named after those proposals. WebAssembly proposals go through a phased design process similar to JS and once a feature reaches "phase 4" it's effectively done and ready for everyone to use. That's when browsers start shipping the feature ungated for example. There is no "standard" for what to call these proposals beyond "toolchain conventions" which, at this time, is "whatever LLVM called it". In that sense the choice of feature name here matches what one would use on the command line for C/C++ when compiling WebAssembly. | |
> | |
> This particular proposal, along with others, as pointed out, is "stage 5" which means it's long since done and overdue for stabilization on our end. | |
### "RFC: Packages as (optional) namespaces" rfcs#3243 | |
**Link:** https://github.com/rust-lang/rfcs/pull/3243 | |
TC: epage nominated this for us: | |
> I marked this as I-lang-nominated to double check with the lang team that they are ok with the semi-open namespaces concept being added to the language as suggested in the Cargo team meeting. | |
### "dyn Trait comparison should not include the vtable pointer" rust#106447 | |
**Link:** https://github.com/rust-lang/rust/issues/106447 | |
TC: Amanieu has a new proposal about what to do here: | |
> > We did discuss about potentially deprecating `dyn Trait` pointer comparison during the meeting, but it's hard to so because we can't deprecate trait impl. | |
> | |
> I would like to propose the following plan: | |
> | |
> * Change the semantics of dyn trait pointer comparison to only look at the address. This is necessary to ensure that such pointer comparisons produce consistent results rather than depending on codegen unit splitting and how LLVM merges vtables. | |
> * Deprecate comparisons (Eq/Ord) on dyn trait pointers by adding a lint which encourages people to cast the pointer to `*mut u8` first. This makes the intent of such comparisons much clearer. | |
### "patterns: reject raw pointers that are not just integers" rust#116930 | |
**Link:** https://github.com/rust-lang/rust/pull/116930 | |
TC: RalfJ nominated this for us: | |
> @rust-lang/lang this PR does three things: | |
> | |
> * it fixes our old lint that detects match on function pointers and wide pointers to also catch cases where those pointers are hidden inside other types | |
> * it makes that lint warn-by-default (but doesn't yet make it show up in dependencies) | |
> * it makes the lint also trigger when encountering a raw pointer that was not constructed via `int as *const/mut T`, but e.g. via `&something as *const T` -- those allocations don't have guarantees on their ptr equality just like function pointers and vtables, so we should reject them for the same reason. | |
> | |
> We haven't reached a proper conclusion in the recent "match on const" meeting, but the general direction seems to have been towards "it should still work like a pattern" (and not like sugar for `==`), and this PR is a crucial step towards ensuring that they do indeed behave like a pattern, with structural properties and all that. Even if the goal is just to future-proof against both options (as has been the strategy so far), we want this warning. | |
> | |
> Crater found only 3 cases where the warning triggers, in the entire ecosystem. All of them would already warn on stable if they enabled the lint; the new cases covered by this PR were not detected at all by crater. | |
### "[`RFC 3086`] Attempt to try to resolve blocking concerns" rust#117050 | |
**Link:** https://github.com/rust-lang/rust/pull/117050 | |
TC: c410-f3r is looking for feedback about whether the approach he has taken, described in the following link, is the desired approach for resolving the blocking concerns: | |
https://github.com/rust-lang/rust/issues/83527#issuecomment-1744822345 | |
TC: What do we think? | |
### "Fix `non_camel_case_types` for screaming single-words" rust#116389 | |
**Link:** https://github.com/rust-lang/rust/pull/116389 | |
TC: @petrochenkov nominated this for us: | |
> A relaxed version of this PR could be bumping the length limit from 3 letters to 4. E.g. `ABC` would still be considered camel case, but `ABCD`/`ABCDF`/etc not. | |
This is related to [#116336](https://github.com/rust-lang/rust/issues/116336) and [#60570](https://github.com/rust-lang/rust/issues/60570). | |
### "Decision: semantics of the `#[expect]` attribute" rust#115980 | |
**Link:** https://github.com/rust-lang/rust/issues/115980 | |
TC: @nikomatsakis gives this background: | |
> This issue is spun out from #54503 to serve as the decision issue for a specific question. The question is what the 'mental model' for the `expect` attribute should be. Two proposed options: | |
> | |
> 1. The expectation is fulfilled, if a #[warn] attribute in the same location would cause a diagnostic to be emitted. The suppression of this diagnostic fulfills the expectation. ([src](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Expect.20attribute.20mental.20model/near/341522535)) (Current implementation in rustc) | |
> 2. The expectation is fulfilled if removing the `#[expect]` attribute would cause the warning to be emitted. ([src](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Expect.20attribute.20mental.20model/near/354082551)) | |
> | |
> @xFrednet created a [list of use cases](https://hackmd.io/@xFrednet/expect-attr-use-cases) to help with the discussion of these two models; they found both models work equally well, except for [use case 4](https://hackmd.io/@xFrednet/expect-attr-use-cases#Use-case-4-Suppress-lints-from-CI) which would only be possible with the first model. | |
TC: ...and proposes that we adopt option 1. | |
### "types team / lang team interaction" rust#116557 | |
**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. | |
### "`.await` does not perform autoref or autoderef" rust#111546 | |
**Link:** https://github.com/rust-lang/rust/issues/111546 | |
TC: This was nominated for T-lang (and for T-types) by WG-async. @tmandry said: | |
> We discussed this in a recent wg-async meeting ([notes](https://hackmd.io/G6ULofyXSIS4CK9u-jwYRg)). The consensus was that we thought the change was well-motivated. At the same time, we want to be cautious about introducing problems (namely backwards compatibility). | |
> | |
> There should probably be a crater run of this change, and we should also work through any problematic interactions that could be caused by this change. (@rust-lang/types should probably weigh in.) | |
> | |
> The main motivation for the change is the analogy to `.method()`, as well as to wanting async and sync to feel similarly convenient in most cases. | |
> | |
> Note that there is another analogy that works against this, the analogy to `IntoIterator`, where the lang-effect form (`for _ in foo {}`) does not do autoref/autoderef. However, given that this _looks_ very different from `foo.await`, and taking a reference with that form is significantly more convenient (`for x in &foo` or `for x in foo.iter()` vs `(&foo).await`), it seemed the analogy was stretched pretty thin. So we elected to put more weight on the above two considerations. | |
> | |
> That being said, this change would need lang team signoff. You can consider this comment wg-async's official recommendation to the lang team. | |
### "Support overriding `warnings` level for a specific lint via command line" rust#113307 | |
**Link:** https://github.com/rust-lang/rust/pull/113307 | |
TC: We discussed in the 2023-09-26 meeting, but were unsure of the question we were being asked. @jieyouxu has since replied: | |
> I believe I wanted to ask that if the command line indeed forms the root of the tree, or if it actually overrides the source annotations. | |
TC: On that basis, @tmandry replied: | |
> ### Nesting | |
> | |
> I think the command line (specifically `-A`, `-W`, `-D` flags) should form the root of the tree. We have `--cap-lints`, `--force-warn`, and `-F` (forbid) for overriding the source. (Actually the mental model documented in the [rustc book](https://doc.rust-lang.org/rustc/lints/levels.html) is that `force-warn` and `forbid` still form the root of the tree, but cannot be overridden; I think the distinction is mostly academic.) | |
> | |
> That's almost all the expressive power one could want along this axis. One wrinkle is that `--forbid` is overridden by `--cap-lints`, while `--force-warn` is not. If we wanted full fine-grained control we could always add `--force-allow` and `--force-deny`. | |
> | |
> ### `warnings` | |
> | |
> Regarding the meaning of `warnings`, it _is_ a simpler mental model for this to mean "the set of things that are warn-by-default". But this ignores what I perceive to be a common (and valid) use case, which is to disallow _all_ warnings in a codebase: In other words, prevent code from being checked in that causes warnings to be printed to a user's screen. Of course, for this to be practical one must control the version of rustc being used to build a codebase, but that is common in monorepo setups. | |
> | |
> ### Conclusion | |
> | |
> Given that there is an existing use case that relies on documented behavior, I think we should continue to treat `warnings` as a "redirect" for all warnings that come out of a particular level of the tree. Interpreting `-Awarnings -Wfoo` in the way proposed by this PR would muddy the (already complicated) mental model and add inconsistency between CLI and the command line, as noted by @oli-obk. | |
> | |
> A different group, like `default-warnings`, could be used to mean "the set of things that are warn-by-default". The compiler could further warn users that specify `-Awarnings -Wfoo` on the command line to use `-Adefault-warnings -Wfoo` instead. | |
### "Uplift `clippy::precedence` lint" rust#117161 | |
**Link:** https://github.com/rust-lang/rust/pull/117161 | |
TC: The proposal is to lint against: | |
```rust | |
-2.pow(2); // Equals -4. | |
1 << 2 + 3; // Equals 32. | |
``` | |
These would instead be written: | |
```rust | |
-(2.pow(2)); // Equals -4. | |
1 << (2 + 3); // Equals 32. | |
``` | |
Prompts for discussion: | |
- Is this an appropriate lint for `rustc`? | |
- How do other languages handle precedence here? | |
- Is minus special enough to treat differently than other unary operators? | |
### "Implement `PartialOrd` and `Ord` for `Discriminant`" rust#106418 | |
**Link:** https://github.com/rust-lang/rust/pull/106418 | |
TC: We discussed in the 2023-09-26 meeting. We decided to simply let T-libs-api know about what we had discussed. Niko [left a comment](https://github.com/rust-lang/rust/pull/106418#issuecomment-1736377423) to the effect that "right now we seem to have no good options." | |
TC: There has since been further discussion on the issue. E.g., Amanieu noted: | |
> While discussing this in the libs-api meeting today, we noticed that the documentation for `mem::discriminant` [doesn't guarantee stability for the discriminant](https://doc.rust-lang.org/nightly/std/mem/fn.discriminant.html#stability). This means that the actual ordering of `Discriminant` is _not_ tied to the source code order of the variants. | |
> | |
> This makes it unsuitable for many use cases, including the one in the PR description, which is to manually implement `Ord` for an enum. To enable this use case, we need to guarantee that the ordering of `Discriminant` matches that of the variants in the source code. | |
TC: Do we have any further thoughts here? | |
### "Stabilize C string literals" rust#117472 | |
**Link:** https://github.com/rust-lang/rust/pull/117472 | |
TC: This needs a stabilization report. It's probably worth waiting on that to discuss. | |
### "Exhaustiveness: reveal opaque types properly" rust#116821 | |
**Link:** https://github.com/rust-lang/rust/pull/116821 | |
TC: Oli nominated this one for us: | |
> Nominating for T-lang discussion, as this allows more code to compile (both on stable, but especially on nightly around type-alias-impl-trait). | |
TC: Nadrieril describes the change as: | |
> Previously, exhaustiveness had no clear policy around opaque types. In this PR I propose the following policy: within the defining scope of an `impl Trait` opaque type, exhaustiveness uses the real type. From what I can tell, this doesn't change anything for non-empty types. | |
> | |
> I'm not sure how consistent this is with other operations allowed on opaque types; I believe this will require FCPs. | |
> | |
> The observable changes are: | |
> | |
> * when the real type is uninhabited, matches within the defining scopes can now rely on that for exhaustiveness, e.g.: | |
> | |
> ```rust | |
> #[derive(Copy, Clone)] | |
> enum Void {} | |
> fn return_never_rpit(x: Void) -> impl Copy { | |
> if false { | |
> match return_never_rpit(x) {} | |
> } | |
> x | |
> } | |
> ``` | |
> | |
> * this properly fixes ICEs like [ICE: `Unexpected type for 'Single' constructor` #117100](https://github.com/rust-lang/rust/issues/117100) that occurred because a same match could have some patterns where the type is revealed and some where it is not. | |
> | |
> Bonus subtle point: if `x` is opaque, a match like `match x { ("", "") => {} ... }` will constrain its type ([playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=901d715330eac40339b4016ac566d6c3)). This is not the case for `match x {}`: this will not constain the type, and will only compile if something else constrains the type to be empty. | |
> | |
> Fixes #117100 | |
TC: Let's give some more time for context to develop on the thread before discussing in the meeting. | |
## Action item review | |
- [Action items list](https://hackmd.io/gstfhtXYTHa3Jv-P_2RK7A) | |
## Pending lang team project proposals | |
None. | |
## PRs on the lang-team repo | |
### "Update Auto Traits Freeze Discussion Link" lang-team#231 | |
**Link:** https://github.com/rust-lang/lang-team/pull/231 | |
## RFCs waiting to be merged | |
None. | |
## `S-waiting-on-team` | |
### "Tracking issue for dyn upcasting coercion" rust#65991 | |
**Link:** https://github.com/rust-lang/rust/issues/65991 | |
### "Stabilize `anonymous_lifetime_in_impl_trait`" rust#107378 | |
**Link:** https://github.com/rust-lang/rust/pull/107378 | |
### "remove 'illegal_floating_point_literal_pattern' future-compat lint" rust#116098 | |
**Link:** https://github.com/rust-lang/rust/pull/116098 | |
### "warn less about non-exhaustive in ffi" rust#116863 | |
**Link:** https://github.com/rust-lang/rust/pull/116863 | |
## Proposed FCPs | |
**Check your boxes!** | |
### "RFC: inherent trait implementation" rfcs#2375 | |
**Link:** https://github.com/rust-lang/rfcs/pull/2375 | |
### "unsafe attributes" rfcs#3325 | |
**Link:** https://github.com/rust-lang/rfcs/pull/3325 | |
### "MaybeDangling" rfcs#3336 | |
**Link:** https://github.com/rust-lang/rfcs/pull/3336 | |
### "add float semantics RFC" rfcs#3514 | |
**Link:** https://github.com/rust-lang/rfcs/pull/3514 | |
### "Tracking issue for dyn upcasting coercion" rust#65991 | |
**Link:** https://github.com/rust-lang/rust/issues/65991 | |
### "Stabilise inline_const" rust#104087 | |
**Link:** https://github.com/rust-lang/rust/pull/104087 | |
### "Stabilize `anonymous_lifetime_in_impl_trait`" rust#107378 | |
**Link:** https://github.com/rust-lang/rust/pull/107378 | |
### "Report monomorphization time errors in dead code, too" rust#112879 | |
**Link:** https://github.com/rust-lang/rust/pull/112879 | |
### "FCP process: Require 2/3 majority for FCP" rust#114986 | |
**Link:** https://github.com/rust-lang/rust/issues/114986 | |
### "`c_unwind` full stabilization request: change in `extern "C"` behavior" rust#115285 | |
**Link:** https://github.com/rust-lang/rust/issues/115285 | |
### "Decision: semantics of the `#[expect]` attribute" rust#115980 | |
**Link:** https://github.com/rust-lang/rust/issues/115980 | |
## Active FCPs | |
### "Lifetime Capture Rules 2024" rfcs#3498 | |
**Link:** https://github.com/rust-lang/rfcs/pull/3498 | |
### "TAIT defining scope options" rust#107645 | |
**Link:** https://github.com/rust-lang/rust/issues/107645 | |
### "Stabilize `const_maybe_uninit_zeroed` and `const_mem_zeroed`" rust#116218 | |
**Link:** https://github.com/rust-lang/rust/pull/116218 | |
### "Guarantee that `char` has the same size and alignment as `u32`" rust#116894 | |
**Link:** https://github.com/rust-lang/rust/pull/116894 | |
### "document that the null pointer has the 0 address" rust#116988 | |
**Link:** https://github.com/rust-lang/rust/pull/116988 | |
### "Guarantee that raw pointer conversions preserve slice element count" reference#1417 | |
**Link:** https://github.com/rust-lang/reference/pull/1417 | |
## P-critical issues | |
None. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment