fix fn/const items implied bounds and wf check (rebase) by lcnr · Pull Request #120019 · rust-lang/rust (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation7 Commits4 Checks0 Files changed

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

lcnr

A rebase of #104098, see that PR for discussion. This is pretty much entirely the work of @aliemjay. I received his permission for this rebase.

These are two distinct changes (edit: actually three, see below):

  1. Wf-check all fn item args. This is a soundness fix.
    Fixes function type params are not checked for well-formedness #104005
  2. Use implied bounds from impl header in borrowck of associated functions/consts. This strictly accepts more code and helps to mitigate the impact of other breaking changes.
    Fixes implied bounds from impl header are not used in associated functions/consts #98852
    Fixes Generic param requires bounds on call to function that is already required for calls to the caller #102611

The first is a breaking change and will likely have a big impact without the the second one. See the first commit for how it breaks libstd.

Landing the second one without the first will allow more incorrect code to pass. For example an exploit of #104005 would be as simple as:

use core::fmt::Display;

trait ExtendLt { fn extend(self) -> Box; }

impl<T: Display> ExtendLt<&'static T> for T { fn extend(self) -> Box { Box::new(self) } }

fn main() { let val = (&String::new()).extend(); println!("{val}"); }

The third change is to to check WF of user type annotations before normalizing them (fixes #104764, fixes #104763). It is mutually dependent on the second change above: an attempt to land it separately in #104746 caused several crater regressions that can all be mitigated by using the implied from the impl header. It is also necessary for the soundness of associated consts that use the implied bounds of impl header. See #104763 and how the third commit fixes the soundness issue in tests/ui/wf/wf-associated-const.rs that was introduces by the previous commit.

r? types

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Jan 16, 2024

@lcnr lcnr added disposition-merge

This issue / PR is in PFCP or FCP with a disposition to merge it.

finished-final-comment-period

The final comment period is finished for this PR / Issue.

T-types

Relevant to the types team, which will review and decide on the PR/issue.

and removed T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Jan 16, 2024

@lcnr

@bors rollup=never

I've added a new known-bug test for #84591 in my rebase and removed tests/ui/wf/wf-in-fn-type-implicit.rs, given that this is the same test as tests/ui/borrowck/fn-item-check-type-params.rs.

All other changes were already reviewed by me in #104098. This change has been FCP'd in #104098 (comment) and will probably cause a minor perf regression #104098 (comment). This is expected as it does strictly more work

@lcnr lcnr mentioned this pull request

Jan 16, 2024

@BoxyUwU

@bors

📌 Commit 66090ef has been approved by BoxyUwU

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jan 16, 2024

@bors

@bors

@rust-timer

Finished benchmarking commit (6bf600b): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) 0.3% [0.2%, 0.5%] 26
Regressions ❌ (secondary) 1.3% [0.3%, 1.6%] 9
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) 0.3% [0.2%, 0.5%] 26

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 1.7% [0.7%, 4.4%] 4
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) -6.0% [-6.0%, -6.0%] 1
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 2.2% [2.2%, 2.2%] 2
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 663.655s -> 665.541s (0.28%)
Artifact size: 308.29 MiB -> 308.32 MiB (0.01%)

@Kobzol

This was a fix that had an expected performance regression, marking as triaged.

@rustbot label: +perf-regression-triaged

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Mar 29, 2024

@he32

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Oct 13, 2024

@he32

This is based on the pkgsrc-wip rust180 package, retaining the main pkgsrc changes as best as I could.

Pkgsrc changes:

Upstream chnages:

Version 1.80.1 (2024-08-08)

Version 1.80.0 (2024-07-25)

Language

Compiler

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

Version 1.79.0 (2024-06-13)

Language

Compiler

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Rustdoc

Misc

Compatibility Notes

Version 1.78.0 (2024-05-02)

Language

Compiler

Target changes:

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

These APIs are now stable in const contexts:

Cargo

Misc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

Version 1.77.0 (2024-03-21)

Compiler

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

Stabilized APIs

Cargo

Rustdoc

Misc

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.