nightly-2021-12-02 to nightly-2021-12-03 regression (PR #91354): auto-deref on Cow does not seem to work anymore in all circumstances

[sdroege]

Code

See https://github.com/gtk-rs/gtk-rs-core/blob/master/glib-macros/tests/test.rs#L290

This fails with the latest nightly with (run cargo test inside the glib-macros directory).

error[E0599]: no method named `as_str` found for enum `Cow<'static, VariantTy>` in the current scope
   --> glib-macros/tests/test.rs:290:48
    |
290 |     assert_eq!(Variant1::static_variant_type().as_str(), "(si)");
    |                         

Simplified code from gtk-rs / glib which works without problems for some reason so I'm not entirely sure what causes it:

pub struct VariantType;
pub struct VariantTy;

impl Borrow<VariantTy> for VariantType {
    fn borrow(&self) -> &VariantTy {
        todo!()
    }
}

impl ToOwned for VariantTy {
    type Owned = VariantType;

    fn to_owned(&self) -> VariantType {
        todo!()
    }
}

impl VariantTy {
    pub fn as_str(&self) -> &str {
        todo!()
    }
}

let x = Cow::Owned(VariantType);
println!("{}", x.as_str());

Version it worked on

It most recently worked on: 2021-12-02

Version with regression

rustc --version --verbose:

rustc 1.59.0-nightly (acbe4443c 2021-12-02)
binary: rustc
commit-hash: acbe4443cc4c9695c0b74a7b64b60333c990a400
commit-date: 2021-12-02
host: x86_64-unknown-linux-gnu
release: 1.59.0-nightly
LLVM version: 13.0.0

(note that this is actually the one from today, 2021-12-03, but there's an off-by-one in the version information)

Running cargo bisect-rustc yields:

searched nightlies: from nightly-2021-12-01 to nightly-2021-12-03
regressed nightly: nightly-2021-12-03
searched commits: from https://github.com/rust-lang/rust/commit/48a5999fceeea84a8971634355287faa349909d4 to https://github.com/rust-lang/rust/commit/acbe4443cc4c9695c0b74a7b64b60333c990a400
regressed commit: https://github.com/rust-lang/rust/commit/18bb8c61a975fff6424cda831ace5b0404277145

Which would be this PR: #91354 (CC @fee1-dead).

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

[spastorino]

Would be nice if we can have a minimal test case for this, so we can reintroduce @fee1-dead changes again being sure that this won't be an issue again.

[sdroege]

Yeah I tried but I don't see the difference between my (incomplete) testcase above and the real code that fails :)

[SNCPlay42]

Reduced: (playground)

use std::borrow::Borrow;
use std::borrow::Cow;

pub struct VariantType {}
pub struct VariantTy {}

impl Borrow<VariantTy> for VariantType {
    fn borrow(&self) -> &VariantTy {
        unimplemented!()
    }
}

impl ToOwned for VariantTy {
    type Owned = VariantType;
    fn to_owned(&self) -> VariantType {
        unimplemented!()
    }
}

impl VariantTy {
    pub fn as_str(&self) -> () {}
}

static _TYP: () = {
    let _ = || {
        Cow::Borrowed(&VariantTy {}).as_str();
    };
};

The existence of _TYP seems to cause as_str to not be found on Cow<'_, VariantTy> everywhere in the crate, i.e. if you add a function like

fn foo(x: Cow<'_, VariantTy>) {
    x.as_str();
}

It will error iff _TYP is present.

[spastorino]

@SNCPlay42 do you want to send a PR with that test case? cc me and I can merge it. Would need to sit on top of this #91491 but it will be on master soon.

[fee1-dead]

I would imagine this error is because of caching still. Because the only const impl in the standard library with a ~const bound is the deref method on Cow. Looking very similar to #89432 (comment).

EDIT: doesn't seem to be at all related to caching, I think this is a general problem with closures in consts and how they interact with const_trait_impl.

[SNCPlay42]

I think this is a general problem with closures in consts

FYI the closure is only there to make this work on stable (and because the original crate had it), this version has the same problem:

#![feature(const_deref)]
#![feature(const_trait_impl)]

use std::borrow::Borrow;
use std::borrow::Cow;

pub struct VariantType {}
pub struct VariantTy {}

impl Borrow<VariantTy> for VariantType {
    fn borrow(&self) -> &VariantTy {
        unimplemented!()
    }
}

impl ToOwned for VariantTy {
    type Owned = VariantType;
    fn to_owned(&self) -> VariantType {
        unimplemented!()
    }
}

impl VariantTy {
    pub const fn as_str(&self) -> () {}
}

static _TYP: () = {
    Cow::Borrowed(&VariantTy {}).as_str();
};

[fee1-dead]

That is expected to not work because the impl of Deref on Cow is expressed like this:

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_deref", issue = "88955")]
impl<B: ?Sized + ToOwned> const Deref for Cow<'_, B>
where
    B::Owned: ~const Borrow<B>,

Your code is in a const context, and the trait bounds are actually unsatisfied.

It actually works if you change impl Borrow to impl const Borrow.

[SNCPlay42]

It's odd that it compiles on yesterday's nightly then (per godbolt) - I guess ~const bounds aren't fully implemented?

On the current nightly it errors with the same no method named `as_str` found for enum `Cow` in the current scope message and causes all other calls (in non-const contexts) to produce the same error, which is what makes me think the closure itself isn't the problem but just necessary to make the code compile.

[fee1-dead]

umm, how do I explain this 😅

We have a cache for whether trait bounds satisfy for a given type for performance, but a bug causes the compiler to treat a closure's body inside const contexts as const specifically for solving whether trait bounds satisfy (rustc_trait_selection). I think adding a call before the usage in static will not error.

EDIT: except it does because static items are checked before other items? I was just speculating but the bug IS related to the closure in the const.

[fee1-dead]

Actually, the closure in const is a bug introduced by the PR that caused the regression.

[apiraino]

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-high

[pnkfelix]

@rustbot assign @fee1-dead