False positive `non-primitive cast` error in recent nightly #18047

frxstrem · 2024-09-04T12:36:33Z

When working for a while in a large codebase, I get a lot of errors like this one from tracing::info! macro invocations:

non-primitive cast: &Arguments<_> as &dyn Value

This seems to come from an expression &(format_args!(...)) as &dyn Value in the macro expansion, which should work (and works according to rustc). std::fmt::Arguments<'_> implements tracing_core::field::Value, however rust-analyzer seems to in some cases still produce an error.

I've so far been unable to create a minimal reproducible example, I've only so far seen this in a fairly large (170k LOC) commercial codebase that I'm working with. Restarting rust-analyzer does make the error go away for a while, in my case for about 1.5 minutes after other errors are visible.

let _ = &format_args!("...") as &dyn std::io::Write; // <-- immediately has error
let _ = &format_args!("...") as &dyn tracing::Value; // <-- has an error after a while

The errors started showing up after updating the rust-analyzer extension earlier today, not sure what the version prior to the update was. The errors seem to not be present on the latest stable release (v0.3.2096). It seems like it may be related to the recently merged #17984.

rust-analyzer version: rust-analyzer version: 0.4.2098-standalone

rustc version: rustc 1.80.1 (3f5fd8dd4 2024-08-06)

editor or extension: VSCode, extension version v0.4.2098 (pre-release)

relevant settings: none that I can find

The text was updated successfully, but these errors were encountered:

lnicola · 2024-09-04T12:44:57Z

CC #11847. We might have to mark it as experimental 😞.

ShoyuVanilla · 2024-09-05T00:26:46Z

I've only so far seen this in a fairly large (170k LOC) commercial codebase that I'm working with.

Is your codebase private?

ShoyuVanilla · 2024-09-05T02:14:13Z

CC #11847. We might have to mark it as experimental 😞.

This seems to be caused from coerce_unsize as you said. I'll try with the way rustc handles this 🤔

flodiebold · 2024-09-05T06:24:38Z

This seems to be caused from coerce_unsize as you said. I'll try with the way rustc handles this 🤔

Our CoerceUnsized solving sadly isn't quite correct and probably can't be fixed before the new trait solver is integrated :/

ShoyuVanilla · 2024-09-05T06:34:43Z

Our CoerceUnsized solving sadly isn't quite correct and probably can't be fixed before the new trait solver is integrated :/

It might be turned out otherwise, but I guess it could be.
The rustc's coerce_unsized solving is ten years old(https://github.com/rust-lang/rust/pull/24619/files#diff-12aa26d34290be9b45ffddf48028dd5af1991fbc8a069414b3ecc250cf48dd0aR295) and I think that this is not so much dependent on sophisticated trait solving infra; it mostly does things by itself I think.
I guess it might work well in r-a here if we adopt that "registering unsolved obligation as InferOk's obligations" strategy.

flodiebold · 2024-09-05T07:02:39Z

The rustc's coerce_unsized solving is ten years old(https://github.com/rust-lang/rust/pull/24619/files#diff-12aa26d34290be9b45ffddf48028dd5af1991fbc8a069414b3ecc250cf48dd0aR295) and I think that this is not so much dependent on sophisticated trait solving infra; it mostly does things by itself I think. I guess it might work well in r-a here if we adopt that "registering unsolved obligation as InferOk's obligations" strategy.

It uses trait selection (the selcx.select call), which is part of the trait solving infrastructure and something that Chalk doesn't provide access to (but the new trait solver will). Of course it would be possible to replicate that in RA manually, but I'm not sure that's worth it.

ShoyuVanilla · 2024-09-05T07:13:41Z

The rustc's coerce_unsized solving is ten years old(https://github.com/rust-lang/rust/pull/24619/files#diff-12aa26d34290be9b45ffddf48028dd5af1991fbc8a069414b3ecc250cf48dd0aR295) and I think that this is not so much dependent on sophisticated trait solving infra; it mostly does things by itself I think. I guess it might work well in r-a here if we adopt that "registering unsolved obligation as InferOk's obligations" strategy.

It uses trait selection (the selcx.select call), which is part of the trait solving infrastructure and something that Chalk doesn't provide access to (but the new trait solver will). Of course it would be possible to replicate that in RA manually, but I'm not sure that's worth it.

Rustc branches on selcx.selext result as Ok(None), Err(..)(actually Err branches are two but they are doing error reports anyway) and Ok(Some(..)). Couldn't we do the similar things with db.trait_solve call result as Some(Solution(Ambig)), None and Some(Solution(Unique))?

flodiebold · 2024-09-05T07:19:31Z

No, select isn't the same as full trait solving.

ShoyuVanilla · 2024-09-05T07:20:37Z

No, select isn't the same as full trait solving.

Oh, I see 😢

ShoyuVanilla · 2024-09-05T07:33:30Z

So, we have another reason(one of so many) to quickly migrate into next-gen solver. I hope we could do so in near future.
What would be the proper makeshift fix for this btw? 🤔 Some options I can think up are;

Marking cast diagnostics as an experimental feature(so, not triggered by default)
Marking the whole cast checks as an experimental feature
Ignore cast errors when we are casting into a DST if they are not so obvious

frxstrem · 2024-09-05T08:10:09Z

I've only so far seen this in a fairly large (170k LOC) commercial codebase that I'm working with.

Is your codebase private?

@ShoyuVanilla yes, it's private so unfortunately I haven't reproduced it with anything I can share here.

lnicola · 2024-09-05T11:31:23Z

What would be the proper makeshift fix for this btw?

My preference would be 3 if it's not too hard to implement, otherwise 1?

ShoyuVanilla · 2024-09-05T13:18:54Z

I've only so far seen this in a fairly large (170k LOC) commercial codebase that I'm working with.

Is your codebase private?

@ShoyuVanilla yes, it's private so unfortunately I haven't reproduced it with anything I can share here.

No worries. Could you please tell me whether these false positive diagnostics still exist once the makeshift fix has been released? I'll mention you here when the nightly release done.

ShoyuVanilla · 2024-09-05T13:19:34Z

What would be the proper makeshift fix for this btw?

My preference would be 3 if it's not too hard to implement, otherwise 1?

Mine, too 😄 Then I'll try with it

ShoyuVanilla · 2024-09-06T01:39:04Z

The root cause of this might be a bit different from #11847 as we are calling table.fallback_if_possible() before cast checks at here;

rust-analyzer/crates/hir-ty/src/infer.rs

Lines 712 to 725 in 124c748

    
           table.fallback_if_possible(); 
        
           // Comment from rustc: 
        
           // Even though coercion casts provide type hints, we check casts after fallback for 
        
           // backwards compatibility. This makes fallback a stronger type hint than a cast coercion. 
        
           let mut apply_adjustments = |expr, adj| { 
        
               expr_adjustments.insert(expr, adj); 
        
           }; 
        
           let mut set_coercion_cast = |expr| { 
        
               coercion_casts.insert(expr); 
        
           }; 
        
           for cast in deferred_cast_checks.iter_mut() { 
        
               if let Err(diag) = 
        
                   cast.check(&mut table, &mut apply_adjustments, &mut set_coercion_cast)

The following test just passes in our current revision;

    #[test]
    fn cast_into_dst() {
        check_diagnostics(
            r#"
//- minicore: fmt, builtin_impls, coerce_unsized, dispatch_from_dyn
struct Box<T: ?Sized>(*const T);

impl<T, U> core::ops::CoerceUnsized<Box<U>> for Box<T>
where
    T: Unsize<U> + ?Sized,
    U: ?Sized,
{
}

impl<T: ?Sized> Box<T> {
    fn new(_: T) -> Self {
        loop {}
    }
}

fn test() {
    let _ = &42 as &dyn core::fmt::Debug;
    let _ = Box::new(7) as Box<dyn core::fmt::Debug>;
    let _ = Box::new([4]) as Box<[i32]>;
}
"#,
        )
    }

I'll look into this more 🧐

ShoyuVanilla · 2024-09-06T07:48:38Z

Not sure since I coundn't reproduce this but this seems to be more complicated than first expectations.
Here's some of my observations.

The following is the definition of std::fmt::Arguments

#[lang = "format_arguments"]
#[stable(feature = "rust1", since = "1.0.0")]
#[derive(Copy, Clone)]
pub struct Arguments<'a> {
    // Format string pieces to print.
    pieces: &'a [&'static str],

    // Placeholder specs, or `None` if all specs are default (as in "{}{}").
    fmt: Option<&'a [rt::Placeholder]>,

    // Dynamic arguments for interpolation, to be interleaved with string
    // pieces. (Every argument is preceded by a string piece.)
    args: &'a [rt::Argument<'a>],
}

This struct doesn't have any generic parameter other than lifetime one 'a, so it's very unlikely that it can be casted to &dyn Value somewhere (as I try to reproduce it) and cannot be casted to in other places (in your codebase). I guess r-a might be failing to import some dependencies or other issues around this.

Your example code might be another problem.

let _ = &format_args!("...") as &dyn std::io::Write; // <-- immediately has error
let _ = &format_args!("...") as &dyn tracing::Value; // <-- has an error after a while

The first one immediately has error as you said, and the second one took a while to show up an error -> this is the clue.

I think that the first one is from the rust-analyzer's native diagnostics - and this is not false positive because Arguments doesn't implement Write -, i.e. the very diagnostics that made with rust-analyzer's own logic, and the second one is from the flycheck error, that rust analyzer runs cargo check (or cargo clippy if your setting is so) in the background and passes its output to your text editor.
The former is releatively faster because it re-uses unchaged part of the codebase that parsed by rust-analyzer and the later may take some time because it just runs cargo check or cargo clippy.

Of course, it might be the case that just second one took more time to evaluate but if the native-flycheck difference is the case, your example code is another issue that has something to do with flycheck.

To verify this, you may test with just typing the second line let _ = &format_args!("...") as &dyn tracing::Value; without saving if you are using default rust-analyzer options(because saving triggers flycheck) or setting rust-analyzer.checkOnSave as false to disable flycheck on save

frxstrem · 2024-09-09T14:22:02Z

So I poked around in the code a bit more, and managed to actually get a reproducible example in a separate workspace:

use tonic::{body::BoxBody, client::GrpcService};

pub fn foo<S>()
where
    S: GrpcService<BoxBody>,
    S::ResponseBody: 'static, // <--- key line
{
    let _ = &format_args!("...") as &dyn tracing::Value;
}

(using tonic 0.12.2)

When the marked line is commented out, the code compiles fine, but when it's present rust-analyzer gives the error. In both cases cargo clippy and cargo check produce no errors.

Running with a slightly updated RA pre-release this time, v0.4.2103.

ShoyuVanilla · 2024-09-09T14:26:41Z

Oh, it would be really helpful to fix this issue. Thanks!
Until this is fixed it might be convenient to disable those false positives with rust-analyzer.diagnostics.disabled
https://rust-analyzer.github.io/manual.html#diagnostics

RalfJung · 2024-09-09T16:46:05Z

Miri also hits this incorrect diagnostics, in this line.

ShoyuVanilla · 2024-09-09T17:12:17Z

Minimized with no dependency other than std
(Works fine with other type alias Error definitions)

trait Foo {
    type Error;
    type Assoc;
}

trait Bar {
    type Error;
}

type Error = Box<dyn std::error::Error>;

trait Baz {
    type Assoc: Bar;
    type Error: Into<Error>;
}

impl<T, U> Baz for T
where
    T: Foo<Assoc = U>,
    T::Error: Into<Error>,
    U: Bar,
    <U as Bar>::Error: Into<Error>,
{
    type Assoc = U;
    type Error = T::Error;
}

struct S;
trait Trait {}
impl Trait for S {}

fn test<T>()
where
    T: Baz,
    T::Assoc: 'static
{
    let _ = &S as &dyn Trait;
          //^^^^^^^^^^^^^^^^ ERROR
}

ShoyuVanilla · 2024-09-09T17:59:06Z

Running rust-analyzer analysis-stats here overflows stack, even the previous versions before #17984 . Interesting 🤔

Database loaded:     682.30ms, 10mb (metadata 371.19ms, 140kb; build 45.06ms, 16kb)
  item trees: 1
Item Tree Collection: 995.80µs, 0b
  crates: 1, mods: 1, decls: 13, bodies: 3, adts: 2, consts: 0
Item Collection:     3.89s, 144mb
Body lowering:       484.42ms, 45mb     
1/3 33% processing: tracing-infer::test
thread 'main' has overflowed its stack

and CHALK_DEBUG="debug" rust-analyzer diagnostics . &> diag.log output is gigabytes long 🤣

Maybe we are messing up with chalk for just simple coercion with complecated trait env.

A line from the log;

generalize_ty ty=Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Bar::Error<[?0 := Foo::Error<[?0 := !0_0]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>]>, universe_index=U0, variance=Invariant

So, my hypothesis is;

We try to coerce &S into &dyn Trait while cast check
Even though the coercion is straitforward, the chalk runs into an infinite recursion due to some recursiveness in trait environment of the function signature and fails due to the chalk recursion limit
So, the coercion fails - this is false positiveness

I think that the miri one is another thing though 🤔

Veykril · 2024-09-10T06:13:24Z

We are likely not spawning a bigger stack thread for analysis-stats (the server runs on a stack with a bigger thread usually) hence the overflow.

ShoyuVanilla · 2024-09-10T12:21:43Z

Further testing shows indeterministic results 🤔

fn main() {
    println!("Hello, world!");
}
pub trait From1<T>: Sized {
    fn from(_: T) -> Self;
}
pub trait Into1<T>: Sized {
    fn into(self) -> T;
}

impl<T, U> Into1<U> for T
where
    U: From1<T>,
{
    fn into(self) -> U {
        U::from(self)
    }
}

impl<T> From1<T> for T {
    fn from(t: T) -> T {
        t
    }
}

trait Foo {
    type ErrorType;
    type Assoc;
}

trait Bar {
    type ErrorType;
}

trait ErrorLike {}

struct BoxLike<T: ?Sized>(*const T);

impl<'a, E> From1<E> for BoxLike<dyn ErrorLike>
where
    E: Trait + 'a,
{
    fn from(_: E) -> Self {
        loop {}
    }
}

type BoxedError = BoxLike<dyn ErrorLike>;

trait Baz {
    type Assoc: Bar;
    type Error: Into1<BoxedError>;
}

impl<T, U> Baz for T
where
    T: Foo<Assoc = U>,
    T::ErrorType: Into1<BoxedError>,
    U: Bar,
    <U as Bar>::ErrorType: Into1<BoxedError>,
{
    type Assoc = U;
    type Error = T::ErrorType;
}
struct S;
trait Trait {}
impl Trait for S {}

fn test<T>()
where
    T: Baz,
    T::Assoc: 'static,
{
    let _ = &S as &dyn Trait;
}

running rust-analyzer diagnostics to the above sometimes fails to cast due to &dyn Trait is not sized here

rust-analyzer/crates/hir-ty/src/infer/cast.rs

Lines 114 to 121 in e35227d

    
           if !self.cast_ty.data(Interner).flags.contains(TypeFlags::HAS_TY_INFER) 
        
               && !table.is_sized(&self.cast_ty) 
        
           { 
        
               return Err(InferenceDiagnostic::CastToUnsized { 
        
                   expr: self.expr, 
        
                   cast_ty: self.cast_ty.clone(), 
        
               }); 
        
           }

and sometimes with coercion failure, as before.

Obviously, the given types and coercions are nothing to do with T here

fn test<T>()
where
    T: Baz,
    T::Assoc: 'static,
{
    let _ = &S as &dyn Trait;
}

but commenting out T::Assoc: 'static bounds makes error gone as frxstrem said.
Funny thing is that chaging T::Assoc: 'static bound to other non-lifetime bounds works like T::Assoc: Default, T::Assoc: Foo but adding lifetime with that as T::Assoc: Default + 'static doesn't works.
Adding T::Assoc: Trait doesn't work either.

Maybe the chalk thinks that lifetime bounds and trait Trait bound should take into account when solving something for Trait and messes things up

wbenny · 2024-09-10T16:05:22Z

Just chiming in to let you know that I have hit this exact bug on stable (that has been released yesterday).

Downgrading VS Code extension to previous version helped.

ShoyuVanilla · 2024-09-10T16:30:21Z

Just chiming in to let you know that I have hit this exact bug on stable (that has been released yesterday).

Downgrading VS Code extension to previous version helped.

I'm preparing a workaround fix for this. Sorry for the inconvenience 😢

Skip checks for cast to dyn traits It seems that chalk fails to solve some obvious goals when there are some recursiveness in trait environments. And it doesn't support trait upcasting yet. rust-lang/chalk#796 This PR just skips for casting into types containing `dyn Trait` to prevent false positive diagnostics like #18047 and #18083

ShoyuVanilla · 2024-09-12T03:11:09Z

I think that this false positives would be hidden in the nightly release with #18093 though it isn't really fixed in the right way(we need next-gen trait solver to fix this)

pythonberg1997 · 2024-09-14T08:44:32Z

Same issue

ShoyuVanilla · 2024-09-14T09:22:15Z

Same issue

Still getting this on nightly release?

kameko · 2024-09-15T23:12:11Z

Got this on stable converting the println!() macros to tracing::info!() in burn-llama, had to downgrade to last version just to be able to use a logger.

ChayimFriedman2 · 2024-10-01T22:03:02Z

@kameko Are you still getting this on stable?

bkonkle · 2024-10-07T21:22:23Z

I was experiencing this issue and had to downgrade for a bit, and I can confirm that the current version has been corrected.

frxstrem added the C-bug Category: bug label Sep 4, 2024

ShoyuVanilla self-assigned this Sep 5, 2024

ShoyuVanilla added A-ty type system / type inference / traits / method resolution A-diagnostics diagnostics / error reporting labels Sep 5, 2024

ShoyuVanilla mentioned this issue Sep 5, 2024

fix: Disable invalid cast diagnostics for casts like &T as &dyn Trait #18054

Closed

ShoyuVanilla mentioned this issue Sep 9, 2024

Trait upcasting is not implemented yet #18083

Open

ShoyuVanilla mentioned this issue Sep 10, 2024

Skip checks for cast to dyn traits #18093

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

False positive `non-primitive cast` error in recent nightly #18047

False positive `non-primitive cast` error in recent nightly #18047

frxstrem commented Sep 4, 2024 •

edited

Loading

lnicola commented Sep 4, 2024

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

ShoyuVanilla commented Sep 5, 2024

flodiebold commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024

flodiebold commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

flodiebold commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

frxstrem commented Sep 5, 2024

lnicola commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

ShoyuVanilla commented Sep 5, 2024

ShoyuVanilla commented Sep 6, 2024 •

edited

Loading

ShoyuVanilla commented Sep 6, 2024

frxstrem commented Sep 9, 2024

ShoyuVanilla commented Sep 9, 2024

RalfJung commented Sep 9, 2024

ShoyuVanilla commented Sep 9, 2024

ShoyuVanilla commented Sep 9, 2024 •

edited

Loading

Veykril commented Sep 10, 2024

ShoyuVanilla commented Sep 10, 2024 •

edited

Loading

wbenny commented Sep 10, 2024 •

edited

Loading

ShoyuVanilla commented Sep 10, 2024

ShoyuVanilla commented Sep 12, 2024

pythonberg1997 commented Sep 14, 2024

ShoyuVanilla commented Sep 14, 2024

kameko commented Sep 15, 2024

ChayimFriedman2 commented Oct 1, 2024

bkonkle commented Oct 7, 2024

False positive non-primitive cast error in recent nightly #18047

False positive non-primitive cast error in recent nightly #18047

Comments

frxstrem commented Sep 4, 2024 • edited Loading

lnicola commented Sep 4, 2024

ShoyuVanilla commented Sep 5, 2024 • edited Loading

ShoyuVanilla commented Sep 5, 2024

flodiebold commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024

flodiebold commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024 • edited Loading

flodiebold commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024 • edited Loading

frxstrem commented Sep 5, 2024

lnicola commented Sep 5, 2024

ShoyuVanilla commented Sep 5, 2024 • edited Loading

ShoyuVanilla commented Sep 5, 2024

ShoyuVanilla commented Sep 6, 2024 • edited Loading

ShoyuVanilla commented Sep 6, 2024

frxstrem commented Sep 9, 2024

ShoyuVanilla commented Sep 9, 2024

RalfJung commented Sep 9, 2024

ShoyuVanilla commented Sep 9, 2024

ShoyuVanilla commented Sep 9, 2024 • edited Loading

Veykril commented Sep 10, 2024

ShoyuVanilla commented Sep 10, 2024 • edited Loading

wbenny commented Sep 10, 2024 • edited Loading

ShoyuVanilla commented Sep 10, 2024

ShoyuVanilla commented Sep 12, 2024

pythonberg1997 commented Sep 14, 2024

ShoyuVanilla commented Sep 14, 2024

kameko commented Sep 15, 2024

ChayimFriedman2 commented Oct 1, 2024

bkonkle commented Oct 7, 2024

False positive `non-primitive cast` error in recent nightly #18047

False positive `non-primitive cast` error in recent nightly #18047

frxstrem commented Sep 4, 2024 •

edited

Loading

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

ShoyuVanilla commented Sep 5, 2024 •

edited

Loading

ShoyuVanilla commented Sep 6, 2024 •

edited

Loading

ShoyuVanilla commented Sep 9, 2024 •

edited

Loading

ShoyuVanilla commented Sep 10, 2024 •

edited

Loading

wbenny commented Sep 10, 2024 •

edited

Loading