Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust-analyser not processing results from next() method of async_std::stream::StreamExt #4243

Closed
SillyMoo opened this issue May 1, 2020 · 22 comments
Assignees
Labels
A-macro macro expansion A-ty type system / type inference / traits / method resolution Broken Window Bugs / technical debt to be addressed immediately E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now

Comments

@SillyMoo
Copy link

SillyMoo commented May 1, 2020

Rust analyser is not able to process the next() method at the end of the attached rust file. This works ok with vs-code and rls, but vs-code with rust-analyzer it fails to return a type for the result of next().

src.zip

@bjorn3
Copy link
Member

bjorn3 commented May 1, 2020

Maybe a duplicate of #1791? (types wrong with async/await)

Content of the zip, for mobile users
use async_amqp::*;
use async_std::stream::StreamExt;
use lapin::{
    message::DeliveryResult, options::*, publisher_confirm::Confirmation, types::FieldTable,
    BasicProperties, Connection, ConnectionProperties, Result,
};
use log::info;
use futures::channel::mpsc;
use futures::sink::SinkExt;

struct TaskProperty {
    name: String,
    sval: Option<String>,
    nval: Option<i32>,
}
struct Task {
    task_id: String,
    task_ulid: String,
    sample_id: String,
    vm_profile: String,
    task_properties: Vec<TaskProperty>,
}

#[async_std::main]
async fn main() -> Result<()> {

    env_logger::init();
    
    let (tx, mut rx)= mpsc::channel::<String>(100);


    let addr = "amqp://localhost:5673";
    let conn = Connection::connect(&addr, ConnectionProperties::default().with_async_std()).await?;
    let queue_name = "bob";
    info!("CONNECTED");

    let channel_a = conn.create_channel().await?;
    let channel_b = conn.create_channel().await?;

    let queue = channel_a
        .queue_declare(
            queue_name,
            QueueDeclareOptions::default(),
            FieldTable::default(),
        )
        .await?;

    info!("Declared queue {:?}", queue);

    let consumer = channel_b
        .clone()
        .basic_consume(
            queue_name,
            "my_consumer",
            BasicConsumeOptions::default(),
            FieldTable::default(),
        )
        .await?;

    consumer.set_delegate(move |delivery: DeliveryResult| {
        let channel_b = channel_b.clone();
        let mut tx = tx.clone();
        async move {
            let delivery = delivery.expect("error caught in in consumer");
            if let Some(delivery) = delivery {
                channel_b
                    .basic_ack(delivery.delivery_tag, BasicAckOptions::default())
                    .await
                    .expect("failed to ack");
                tx.send(String::from_utf8(delivery.data).unwrap()).await.unwrap();
            }
        }
    });

    let payload = b"Hello world!";

    
    let confirm = channel_a
        .basic_publish(
            "",
            queue_name,
            BasicPublishOptions::default(),
            payload.to_vec(),
            BasicProperties::default(),
        )
        .await?
        .await?;
    assert_eq!(confirm, Confirmation::NotRequested);
    let res = rx.next().await.unwrap();
    info!("Got {:?}", res);
    Ok(())
}
[package]
name = "ivm2_rust"
version = "0.1.0"
authors = ["Chris Mills <Chris_Mills@symantec.com>"]
edition = "2018"

[dependencies]
smol = "0.1.4"
log = "0.4.8"
async-amqp = "0.1.0-beta3"
env_logger = "0.7.1"
futures = "0.3.4"

[dependencies.async-std]
version = "^1.0"
features = ["default"]

[dev-dependencies.async-std]
version = "^1.0"
features = ["attributes", "default"]

[dependencies.lapin]
version = "1.0.0-beta3"
default-features = true

@SillyMoo
Copy link
Author

SillyMoo commented May 1, 2020

It's worth noting that the failure is at the call to next() rather than the call to await. So not quite sure if it is the same thing.

@matklad matklad added A-ty type system / type inference / traits / method resolution E-unknown It's unclear if the issue is E-hard or E-easy without digging in labels May 1, 2020
@flodiebold flodiebold added the A-macro macro expansion label May 3, 2020
@flodiebold
Copy link
Member

It looks to me like we're not managing to expand the extension_trait! macro.

@edwin0cheng
Copy link
Member

Yeah, it is related to #4039

@edwin0cheng
Copy link
Member

@SillyMoo #4283 is landed, and it should fix this issue.

@SillyMoo
Copy link
Author

SillyMoo commented May 4, 2020

Great stuff, thanks.

@lnicola
Copy link
Member

lnicola commented Jun 24, 2020

Update: we still don't resolve next.

Or maybe we do, but it doesn't work on my system (#4676).

@lnicola
Copy link
Member

lnicola commented Oct 14, 2020

Triage: this still happens. It looks like we still don't expand that macro correctly:

image

@lnicola
Copy link
Member

lnicola commented Oct 14, 2020

A first step here might be to extract extension_trait! macro and see if we can come up with a smaller reproducible example.

@lnicola
Copy link
Member

lnicola commented Jan 27, 2021

Somewhat minimized test case:

macro_rules! extension_trait {
    (

        pub trait $name:ident {
            $($body_base:tt)*
        }
    ) => {

        #[cfg(feature = "docs")]
        pub trait $name {
            extension_trait!(@doc () $($body_base)*);
        }
    };

    (@doc ($($head:tt)*) $token:tt $($tail:tt)*) => {
        extension_trait!(@doc ($($head)* $token) $($tail)*);
    };

    (@doc ($($head:tt)*)) => { $($head)* };
}

extension_trait! {
    pub trait Stream {
        type Item;
    }
}
// Recursive expansion of extension_trait! macro
// ==============================================

#[cfg(feature = "docs")]pub trait Stream {
  extension_trait!(@doc()type Item;
  );
  
}

@lnicola
Copy link
Member

lnicola commented Jan 27, 2021

CC @edwin0cheng :-)

@lnicola lnicola added the S-actionable Someone could pick this issue up and work on it right now label Jan 27, 2021
@edwin0cheng edwin0cheng self-assigned this Jan 27, 2021
@lnicola
Copy link
Member

lnicola commented Mar 7, 2021

Triage: #4243 (comment) still happens after the recent macro fixes.

@edwin0cheng
Copy link
Member

It is because we ignore macro expansion under #[cfg(feature = "docs")] if that feature is off.

@edwin0cheng
Copy link
Member

edwin0cheng commented Mar 8, 2021

This rabbit hole is quite deep :)

The actual problem of extension_trait! macro are the following lines:

        // When not rendering docs, re-export the base trait from the futures crate.
        #[cfg(not(feature = "docs"))]
        pub use $base as $name;

        // The extension trait that adds methods to any type implementing the base trait.
        #[doc = $doc_ext]
        pub trait $ext: $name {
            extension_trait!(@ext () $($body_ext)*);
        }

And noted that by default, #[cfg(not(feature="docs"))] will be false in our RA default cfg handling code, but #[cfg(feature = "docs"] will also failed to expand as I mentioned before.

Such that, there are no code to implement the next() method, so it cannot be inference.

@lnicola
Copy link
Member

lnicola commented Mar 8, 2021

And noted that by default, #[cfg(not(feature="docs"))] will be false in our RA default cfg handling code

Wait, why? Is that how it's supposed to work?

@jonas-schievink
Copy link
Contributor

The expansion in #4243 (comment) looks correct to me. After all, the #[cfg] is disabled there, and I think this matches rustc behavior (not that it matters, since the entire trait is cfgd out).

@jonas-schievink
Copy link
Contributor

Removing the #[cfg] or adding not(...) makes it expand like I'd expect, producing pub trait Stream { type Item; }.

@PorkSausages
Copy link

I don't really understand what's being said in this thread, how do I configure rust-analyzer to work properly in this instance if it has been fixed?

@lnicola
Copy link
Member

lnicola commented Dec 20, 2021

@PorkSausages it hasn't been fixed and I'm still not sure what's wrong. async-std uses a pretty complicated macro to define its own documentation for Stream, while re-exporting StreamExt trait from the futures crate. We seem to not expand that macro correctly, so we don't know what Stream is.

It was suggested above that this might be caused by some #[cfg(not(feature = "docs"))] attributes, but it doesn't work for me after removing them.

@lnicola lnicola added the Broken Window Bugs / technical debt to be addressed immediately label Dec 20, 2021
@jonas-schievink
Copy link
Contributor

If I remove enough tokens from the StreamExt trait, it eventually expands successfully. Some logging confirms that we are running into the recursion limit here:

https://github.com/rust-analyzer/rust-analyzer/blob/9ba6cfa9c70c153b71982b07f39aab73a5252ef8/crates/hir_def/src/data.rs#L380-L382

This happens because extension_trait! is internally using a token muncher to consume the input, and the input is very large.

@jonas-schievink
Copy link
Contributor

async-std sets #![recursion_limit = "2048"], so I'd say it's expected that this doesn't work in rust-analyzer.

@lnicola
Copy link
Member

lnicola commented Jan 28, 2022

Fixed in #11360.

Unfortunately, our macro expansion code is somewhat slow, so performance might not be ideal when working on such code: #11363.

@lnicola lnicola closed this as completed Jan 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion A-ty type system / type inference / traits / method resolution Broken Window Bugs / technical debt to be addressed immediately E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

8 participants