Skip to content

fix fn/const items implied bounds and wf check#104098

Closed
aliemjay wants to merge 4 commits intorust-lang:masterfrom
aliemjay:fn-wf
Closed

fix fn/const items implied bounds and wf check#104098
aliemjay wants to merge 4 commits intorust-lang:masterfrom
aliemjay:fn-wf

Conversation

@aliemjay
Copy link
Copy Markdown
Contributor

@aliemjay aliemjay commented Nov 7, 2022

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<Witness> {
    fn extend(self) -> Box<dyn Display>;
}

impl<T: Display> ExtendLt<&'static T> for T {
    fn extend(self) -> Box<dyn Display> {
        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.

cc @lcnr
r? types

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. perf-regression Performance regression. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-types Relevant to the types team, which will review and decide on the PR/issue.

Projects

None yet