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

Inlay hints should work in attributed items with bodies #10043

Closed
Tracked by #9868
Veykril opened this issue Aug 26, 2021 · 8 comments · Fixed by #10231 or #10274
Closed
Tracked by #9868

Inlay hints should work in attributed items with bodies #10043

Veykril opened this issue Aug 26, 2021 · 8 comments · Fixed by #10231 or #10274
Labels
A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now

Comments

@Veykril
Copy link
Member

Veykril commented Aug 26, 2021

Currently attributes turn these items into macro expansion so our inlay hints disappear from function bodies and the like.

@Veykril Veykril added A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now labels Aug 26, 2021
@jonas-schievink
Copy link
Contributor

Inlay hints should work inside all macro invocations if they are unambiguous, I'd say

@Veykril
Copy link
Member Author

Veykril commented Aug 26, 2021

I wouldn't say so, there are plenty of macros where showing inlay hints would feel out of place since a lot of them are used for domain specific language purposes. Having inlay hints show up in those might disrupt that.

Attributes on the other hand always take in full items which are valid rust syntax so showing them there at all times shouldn't have problems.

@jhgg
Copy link
Contributor

jhgg commented Sep 19, 2021

Hmm, still broken in this situation, using actix-web = "4.0.0-beta.8"

2021-09-19_04-08-10

use std::io;

use actix_web::{web, App, HttpServer};

#[actix_web::main]
async fn main() -> io::Result<()> {
    HttpServer::new(move || {
        App::new()
            .route("/foo", web::get().to(handle_health))
            .default_service(web::to(handle_default))
    })
    .bind("0.0.0.0:9090")?
    .run()
    .await
}

async fn handle_health() -> &'static str {
    "ok"
}

async fn handle_default() -> &'static str {
    "ok"
}

@Veykril
Copy link
Member Author

Veykril commented Sep 19, 2021

Looks like we expand #[actix_web::main] which produces

#[actix_web::rt::main(system = "::actix_web::rt::System")]
async fn main() -> io::Result<()>{
  HttpServer::new(move| |{
    App::new().route("/foo",web::get().to(handle_health)).default_service(web::to(handle_default))
  }).bind("0.0.0.0:9090")? .run().await
}

but then we fail to expand the actix_web::rt::main.

@Veykril
Copy link
Member Author

Veykril commented Sep 19, 2021

Interesting, with the expanded code there pasted into a file it worksthe attribute still doesnt expand, it just typechecks now I suppose

@Veykril
Copy link
Member Author

Veykril commented Sep 19, 2021

Ah, actix_web::rt::main is gated behind a cfg(not(test)) so we don't resolve that currently.
Hmm even when it expands the inlays dont work

@Veykril Veykril reopened this Sep 19, 2021
@Veykril
Copy link
Member Author

Veykril commented Sep 19, 2021

I guess this will require #9403, not sure whats wrong but I assume the node down mapping for attributes doesn't work as well as I'd hoped

@Veykril
Copy link
Member Author

Veykril commented Oct 25, 2021

So this actually does work, at least on current rustc stable 1.56. It didn't before because we didn't patch the necessary changes for the older proc-macro abi server. #10633 should make it work on the rest.

@Veykril Veykril closed this as completed Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
3 participants