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

Tracking issue for Span::def_site() #54724

Open
alexcrichton opened this issue Oct 1, 2018 · 8 comments
Open

Tracking issue for Span::def_site() #54724

alexcrichton opened this issue Oct 1, 2018 · 8 comments
Assignees
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-macros-2.0 Area: Declarative macros 2.0 (#39412) A-proc-macros Area: Procedural macros B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. Libs-Tracked Libs issues that are tracked on the team's project board. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

alexcrichton commented Oct 1, 2018

This is a tracking issue for the Span::def_site() API. The feature for this is proc_maro_def_site. The def_site span primarily relates to hygiene today in that it's not copy-paste hygiene.

This is likely blocked on larger hygiene reform and more thorny Macros 2.0 issues. I'm not personally clear on what the blockers are at this time, but I wanted to make sure we had a dedicated tracking issue!

cc #45934

@alexcrichton alexcrichton added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-macros-2.0 Area: Declarative macros 2.0 (#39412) C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Oct 1, 2018
@alexcrichton
Copy link
Member Author

This is likely highly related to #54727 as well

@Aaron1011
Copy link
Member

Hygiene serialization was implemented in #72121. This ensures that items with def-site hygiene are not visible from another crate.

@leo60228
Copy link
Contributor

leo60228 commented Jan 9, 2021

What's this blocked on?

@matthewjasper
Copy link
Contributor

This needs more tests, especially for cross-crate cases. Any bugs found from that would have to be fixed and any unexpected behavior would need to be reviewed.

@jonas-schievink jonas-schievink added E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. A-proc-macros Area: Procedural macros labels Jan 16, 2021
@matklad
Copy link
Member

matklad commented Jun 24, 2021

It would be helpful to understand what def_site means, at least in the general terms. I get a type error when thinking about this -- proc macro definition side is in a crate that is compiled for $host, so in the $target (where we actually expand the macro), this span doesn't exist, no?

EDIT: based on my reading of #68716, I think what def_side does for proc macros is essentially introducing a fresh hygiene context. For macro rules, it would also make def-site names available, but for proc-macros def-site is essentially empty.

@joshtriplett joshtriplett added the S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. label Jun 1, 2022
@nikomatsakis
Copy link
Contributor

Reviewed this in our @rust-lang/lang backlog bonanza. We are tagging this as "needs summary" because, while we'd love to see this stabilized, most of us weren't really sure what "def-site" was supposed to mean -- i.e., this needs more than new tests, we need some kind of docs about the intended effect that can be reviewed.

@winksaville
Copy link

winksaville commented Jun 7, 2022

I'm a noob, but recently while experimenting with proc_macro I discovered a specific circumstance where def_site works and call_site doesn't. Here is the repo.

I created 3 proc_macro's. In inner_using_outer_declarations_via_fn where I create do_something using Span::def_site() and in inner_using_outer_declarations_via_temp I create temp using Span::call_site(). Then in outer I invoke the two inner_* macros twice.

use proc_macro::TokenStream;
use proc_macro2::{Ident, Span};
use quote::quote;

#[proc_macro]
pub fn inner_using_outer_declarations_via_fn(_input: TokenStream) -> TokenStream {
    //let do_something = Ident::new("do_something", Span::call_site()); // Enabling causes compile error
    let do_something = Ident::new("do_something", Span::def_site());
    quote!(
        println!("inner_using_outer_declarations_via_fn:+ a={}", a);
        fn #do_something(a: i32) -> i32 {
            a + 1
        }
        a = #do_something(a);
        println!("inner_using_outer_declarations_via_fn:- a={}", a);
    )
    .into()
}

#[proc_macro]
pub fn inner_using_outer_declarations_via_temp(_input: TokenStream) -> TokenStream {
    let temp = Ident::new("do_something", Span::call_site());
    quote!(
        println!("inner_using_outer_declarations_via_temp:+ a={}", a);
        let #temp = a + 1;
        a = #temp;
        println!("inner_using_outer_declarations_via_temp:- a={}", a);
    )
    .into()
}

#[proc_macro]
pub fn outer(_input: TokenStream) -> TokenStream {
    let q = quote! {
        let mut a = 10;
        inner_using_outer_declarations_via_fn!();
        inner_using_outer_declarations_via_temp!();
        inner_using_outer_declarations_via_fn!();
        inner_using_outer_declarations_via_temp!();
    };
    //println!("outer: q={:#?}", q);
    q.into()
}

In main.rs I simply invoke outer:

use expr_proc_macro_def_site::{
    inner_using_outer_declarations_via_fn, inner_using_outer_declarations_via_temp, outer,
};

fn main() {
    outer!();
}

Running the code works, but you must compile with procmacro2_semver_exempt to enable def_site :

$ RUSTFLAGS='--cfg procmacro2_semver_exempt' cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/debug/expr-proc-macro-def-site`
inner_using_outer_declarations_via_fn:+ a=10
inner_using_outer_declarations_via_fn:- a=11
inner_using_outer_declarations_via_temp:+ a=11
inner_using_outer_declarations_via_temp:- a=12
inner_using_outer_declarations_via_fn:+ a=12
inner_using_outer_declarations_via_fn:- a=13
inner_using_outer_declarations_via_temp:+ a=13
inner_using_outer_declarations_via_temp:- a=14

If you change do_something to use call_site instead:

pub fn inner_using_outer_declarations_via_fn(_input: TokenStream) -> TokenStream {
    let do_something = Ident::new("do_something", Span::call_site()); // Enabling causes compile error
    //let do_something = Ident::new("do_something", Span::def_site());

We now get compile errors, because there are multiple definitions of do_something. So for hygiene, when there can be multiple definitions of an identifier, def_site is needed.

$ cargo run
   Compiling expr-proc-macro-def-site v0.1.0 (/home/wink/prgs/rust/myrepos/expr-proc-macro-def-site)
error[E0428]: the name `do_something` is defined multiple times
 --> src/main.rs:6:5
  |
6 |     outer!();
  |     ^^^^^^^^
  |     |
  |     `do_something` redefined here
  |     previous definition of the value `do_something` here
  |
  = note: `do_something` must be defined only once in the value namespace of this block
  = note: this error originates in the macro `inner_using_outer_declarations_via_fn` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0618]: expected function, found `i32`
 --> src/main.rs:6:5
  |
6 |     outer!();
  |     ^^^^^^^^
  |     |
  |     call expression requires function
  |     `do_something` has type `i32`
  |
  = note: this error originates in the macro `inner_using_outer_declarations_via_fn` (in Nightly builds, run with -Z macro-backtrace for more info)

Some errors have detailed explanations: E0428, E0618.
For more information about an error, try `rustc --explain E0428`.
error: could not compile `expr-proc-macro-def-site` due to 2 previous errors

I'm positive what I've pointed out is known by most people looking at this issue, but for those that aren't experts this might be clarifying. Something like this could be added to the documentation. I'm willing to provide a PR after getting feedback.

@Veykril
Copy link
Member

Veykril commented Sep 17, 2023

Raising for awareness: Def site hygiene has some odd properties with fields (and enum variants) by allowing them to have non-unique names which makes sense, but at the same time yields some very confusing results in regards to diagnostics and more importantly derives (this breaks serde de/serialization in very funny ways),
playground:

#![feature(decl_macro)]
macro m($s:ident, $field:ident) {
    #[derive(Debug, Default)]
    struct $s {
        field: u32,
        $field: u32
    }
}

m!(S, field);

fn main() {
    println!("{:?}", S::default());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-macros-2.0 Area: Declarative macros 2.0 (#39412) A-proc-macros Area: Procedural macros B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. Libs-Tracked Libs issues that are tracked on the team's project board. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests