Accessing private fields from the def site of a proc macro

[antoyo]

Hi.
I'm not sure whether it is a bug or not, that may be an hygiene issue.
But, in the following repository, if you run it with cargo build, you get the following error:

error[E0451]: field `my_field` of struct `MyStruct` is private
  --> src/main.rs:12:5
   |
12 |     pp!(MyStruct);
   |     ^^^^^^^^^^^^^^ field `my_field` is private

However, that does not make sense, because the field is declared in the same module:

struct MyStruct {
    my_field: i32,
}

fn main() {
    pp!(MyStruct); // Error: field `my_field` of struct `MyStruct` is private.
}

This only happens when this feature is enabled.
What makes me think this might be an hygiene issue is that I hard-coded the field name here.
So, if it's a bug, please fix this issue.
Thank you.

[antoyo]

@alexcrichton Since it's using proc-macro2, I cc you.
Do you have any idea for what this issue is about?
Thank you.

[alexcrichton]

This is likely related to the span that's attached to the field as that affects hygiene/resolution. I believe @dtolnay has been working with that more recently than I and may be able to help you out as well

[dtolnay]

Yes this is a span difference between --features proc-macro2/nightly and stable. On stable, all tokens that come out of your proc macro are implicitly spanned with Span::call_site(). Resolving them at the call site means all the private stuff visible from proc-macro-private-bug/src/main.rs is visible to the generated code. But on nightly the tokens emitted by quote! are spanned with Span::def_site() by default which means they resolve as if they were accessing your type from a different scope. You can access private fields from the call site by explicitly spanning the my_field: 0 tokens to resolve at the call site.

extern crate proc_macro2;
use proc_macro2::Span;

let span = Span::call_site();
let my_field = quote_spanned! {span=>
    my_field: 0
};
let result = quote! {
    #path { #my_field }
};

[dtolnay]

@jseyfried this is pretty unfortunate. Is it reasonable to make everything accessible in terms of visibility to the call site also accessible to the def site? Not bringing things in scope, but just some way to relax the visibility logic in this case.

struct MyStruct {
    my_field: i32,
}

fn main() {
    // Expands to quote!($input { my_field: 0 }).
    // Currently requires `my_field: 0` to be spanned call site.
    // Should work without that.
    pp!(MyStruct);
}

[jseyfried]

Sorry for the delay getting to this.

It's only supposed to require my_field to be at the call site; this will be fixed once #48082 lands.

I believe the current behavior is hygienic. That is, if my_field is not visible at the macro definition, then it makes sense to me that it would be a privacy error to use it at the macro definition.

Otherwise, whether or not my_field successfully resolves depends on where the macro is invoked; with hygiene, names at the macro definition are supposed to resolve independently of where the macro is invoked.

Do you (plural) think we could instead make it easier to do things at the call site?
For example, we could add #[escapes()], #[uses()], and/or #[defines()] from #47992 (comment) (or one of the other designs):

#[proc_macro]
#[uses(my_field)] // `quote!` will now expand `my_field` at the call site
fn example(_: TokenStream) -> TokenStream {
    quote! { #path { my_field: 0 } } // this just works now
}

You would be able to opt out of hygiene entirely with #[uses(*)], for example.

That being said, if this case (successful name resolution but privacy violation) becomes a pain point even with better hygiene opt-out ergonomics, I suppose I wouldn't be too opposed to relaxing the rules.

cc @nrc @nikomatsakis

[dtolnay]

I would prefer to relax the rules even if it means the same macro invocation resolves differently depending on whether the struct fields are visible to the call site. In the case of braced struct initializer expressions there isn't a risk of resolving to "the wrong" struct field with a particular name, so making it more likely to just do the right thing seems like the right tradeoff.

// `pp!` expands to `quote!($input { my_field: 0 })` without a `#[uses(my_field)]`

mod m {
    pub struct MyStruct {
        my_field: i32,
    }
    const INNER: MyStruct = pp!(MyStruct); // ok, field is visible
}

const OUTER: m::MyStruct = pp!(m::MyStruct); // fails to compile

[nrc]

I would expect that the macro def would have to do some hygiene manipulation in this case, i.e., it would have to explicitly take the hygiene context from the supplied identifier (the struct name) and apply it to the generated identifier (the field name)

[dtolnay]

What do you see as the advantages of requiring the macro to perform the hygiene manipulation?

[jseyfried]

For declarative macros, I think a macro definition accessing a field it can't see is bad for the same reason that a function accessing a field it can't see would be bad -- it requires you to read the macro definitions for all the macros in the containing module to know where the field is getting used. If a declarative macro opts out of this, I see it as part of the macro's API.

For procedural macros, I think there's a better argument for relaxing the rules here.

I believe the logical conclusion of this would be is to fall back to the resolution at the call site if the resolution of a name at a procedural macro def site fails. This would allow the macro to use names at the call site without opt-out while also allowing the macro to use names at the definition site reliably (regardless of where the macro is invoked) where it wants to.

I believe the above together with defines-by-default is backwards compatible with escapes(*) (i.e. opting out of hygiene everywhere such that the entire expansion resolves at the call site).

[nikomatsakis]

@jseyfried

If a declarative macro opts out of this, I see it as part of the macro's API.
...
For procedural macros, I think there's a better argument for relaxing the rules here.

That's interesting. My intuition would be exactly the opposite. In part because inspecting declarative macros just feels easier than inspecting procedural macros (e.g., a regular ripgrep query through the source would turn up the field name).

This would allow the macro to use names at the call site without opt-out while also allowing the macro to use names at the definition site reliably (regardless of where the macro is invoked) where it wants to.

So overall I guess there are two cases to distinguish (at least, are there more?):

• A macro that generates a struct definition and tries to use its fields.
• A macro that tries to access fields from a struct definition that appears outside of the macro.

The latter is what is happening here, right? And, per our conversation the other day, it seems like the same logic would apply to any "scoped" name resolution, e.g. a path like foo::bar.

That last case is interesting because of privacy:

pub mod some_module:: {
    pub macro a() {
        ::some_module::private_fn();
    }

    pub macro b($n:ident) {
        ::some_module::$n();
    }

    pub macro c($n:path) {
        $n();
    }

    pub macro d($n:expr) {
        $n;
    }

    fn private_fn() { }
}

pub fn main() {
    some_module::a!();
    some_module::b!(private_fn);
    some_module::c!(::some_module::private_fn);
    some_module::d!(::some_module::private_fn());
}

Presumably we want case a to work and case d to error, right? I'm not sure at what point though we draw the line. I think I expect a to work and none of the others, but I could see a case for b working as well. (Based on my understanding of how hygiene works, b would fail.)

In any case, the fact that a works in my example is mildly contra to the struct field resolving, right? And yet I agree with @dtolnay that it feels like the struct field ought to resolve. This may just mean that privacy wants to be more permissive, or perhaps it means that we want to be able to resolve "as if" you were at the definition site or the call site and detect ambiguity? I'm not sure I understand how possible that is -- likely, how likely to lead to deadlock.

[jseyfried]

@nikomatsakis

In part because inspecting declarative macros just feels easier than inspecting procedural macros (e.g., a regular ripgrep query through the source would turn up the field name).

Well, not for declarative macros from external crates, but I see your point. Couldn't you make this same argument to allow functions to access private fields as well though?

I guess my argument is that while it would be nice to require a procedural macro to declare how it is unhygienic, this isn't feasible since it has so much power to subvert hygiene, e.g. it can rename a token from the macro input to generate a name that resolves at the call site.

So overall I guess there are two cases to distinguish (at least, are there more?) ... The latter is what is happening here, right?

Yeah, the latter is the issue we're discussing. The former isn't an issue unless, for example, the macro creates a struct with a field at the def site, and then generates an access to that field at the call site or expects the field to be in scope / visible at the call site.

And, per our conversation the other day, it seems like the same logic would apply to any "scoped" name resolution, e.g. a path like foo::bar.

Agreed. I think it applies to any resolution where privacy can be an issue; that is, everything but lexical resolutions.

That last case is interesting because of privacy:

b!, c! and d! fail today because in all of those cases, private_fn is resolved at the call site, where it is not visible. The idea is since that private_fn is private to some_module, you should only be able to name it in some_module.

it means that we want to be able to resolve "as if" you were at the definition site or the call site and detect ambiguity?

I would prefer instead to fall back to resolving at the call site if the resolution at the def site fails as I proposed in the earlier comment. The rationale is that this still allows you to reliably use items at the macro definition regardless of where the macro is invoked, which is one of the two main purposes of hygiene.

If we try both and detect ambiguity, then a resolution at the def site would be ambiguous if something with that name "happened" to be in scope at the call site. This sort of thing is what hygiene is supposed to avoid.

I'm not sure I understand how possible that is -- likely, how likely to lead to deadlock.

It's not a problem in a procedural macro, since the procedural macro definition is fully resolved and compiled before we start resolving the call site. That is, resolving at the definition site will never be blocked on a resolution at the call site, so there can't be a cycle/deadlock.

This could be more of a problem for declarative macros, but I expect we could come up with reasonable shadowing restrictions to make it work if we wanted to.