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

Proc-macro helper attributes on lifetime and const parameters fail to resolve if a built-in derive macro is present #132561

Closed
PonasKovas opened this issue Nov 3, 2024 · 8 comments · Fixed by #132651
Assignees
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros C-bug Category: This is a bug. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@PonasKovas
Copy link
Contributor

I was making a derive proc macro and stumbled upon this weird error, and was told on the rust discord that this might be a bug. Basically, I have a proc macro that uses a helper attribute like this:

#[proc_macro_derive(Test, attributes(test_helper))]
pub fn derive(input: TokenStream) -> TokenStream {
    ...
}

The #[test_helper] attribute is used on generics - including lifetime generics.

When I use it normally it works fine, but if I try to derive it together with another trait, such as Clone - it fails to compile, saying that #[test_helper] was not found in the scope.

#[derive(Test, Clone)]
struct MyStruct<#[test_helper] 'a> {
    inner: &'a (),
}
error: cannot find attribute `test_helper` in this scope
 --> src/lib.rs:3:19
  |
3 | struct MyStruct<#[test_helper] 'a> {
  |                   ^^^^^^^^^^^

There is no error if I put the attribute on a type generic or on a field.

cargo expand-ed code:

struct MyStruct<#[test_helper] 'a> {
    inner: &'a (),
}
#[automatically_derived]
impl<#[test_helper] 'a> ::core::clone::Clone for MyStruct<'a> {
    #[inline]
    fn clone(&self) -> MyStruct<'a> {
        MyStruct {
            inner: ::core::clone::Clone::clone(&self.inner),
        }
    }
}

I have also made a minimal reproducible example.

Meta

rustc --version --verbose:

rustc 1.84.0-nightly (b3f75cc87 2024-11-02)
binary: rustc
commit-hash: b3f75cc872cfd306860c3ad76a239e719015f855
commit-date: 2024-11-02
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.3

If this is really a bug, I can try to investigate if anyone can help me and point me in the right direction.

@PonasKovas PonasKovas added the C-bug Category: This is a bug. label Nov 3, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 3, 2024
@fmease fmease added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-proc-macros Area: Procedural macros labels Nov 3, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 5, 2024
@fmease
Copy link
Member

fmease commented Nov 5, 2024

Minimal:

rustc a.rs --crate-type=proc-macro:

extern crate proc_macro;

#[proc_macro_derive(M, attributes(x))] 
pub fn m(_: proc_macro::TokenStream) -> proc_macro::TokenStream { Default::default() }

rustc b.rs --crate-type=lib --extern a -L. --edition=2021:

#[derive(a::M, Clone)] struct S<#[x] 'a>(&'a ());
  1. Not only affects lifetime parameters but also const parameters (#[x] won't be found on e.g. const N: usize either). As already mentioned in the issue description, they do resolve on type parameters.
  2. It seems like the second derive macro (above: Clone) has to be a built-in macro. I could reproduce this issue with Clone, PartialEq, Hash, std::marker::ConstParamTy etc. (all of which are built-in macros) but failed to reproduce it with non-built-in ones like "a::N" (i.e., another macro from the same crate, no helpers declared) or "c::M" (i.e., another macro from a different crate, no helpers declared).

@fmease fmease added the S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue label Nov 5, 2024
@fmease fmease changed the title Proc macro helper attribute on lifetime bug Proc-macro helper attributes on lifetime and const parameters fail to resolve if a built-in derive macro is present Nov 5, 2024
@PonasKovas
Copy link
Contributor Author

It seems that the issue is that the built-in macros don't remove the attributes from lifetimes and const generics for some reason, when expanding their implementations, while they do for normal type generics.

Sounds like this should be pretty easy to fix, and I would love to contribute. The issue is probably in the rustc_builtin_macros I assume, but I can't find where the attributes are being removed from the type generics of the item definition.

@fmease
Copy link
Member

fmease commented Nov 5, 2024

The relevant code is here:

GenericParamKind::Lifetime { .. } => param.clone(),
GenericParamKind::Type { .. } => {
// Extra restrictions on the generics parameters to the
// type being derived upon.
let bounds: Vec<_> = self
.additional_bounds
.iter()
.map(|p| {
cx.trait_bound(
p.to_path(cx, self.span, type_ident, generics),
self.is_const,
)
})
.chain(
// Add a bound for the current trait.
self.skip_path_as_bound
.not()
.then(|| cx.trait_bound(trait_path.clone(), self.is_const)),
)
.chain({
// Add a `Copy` bound if required.
if is_packed && self.needs_copy_as_bound_if_packed {
let p = deriving::path_std!(marker::Copy);
Some(cx.trait_bound(
p.to_path(cx, self.span, type_ident, generics),
self.is_const,
))
} else {
None
}
})
.chain(
// Also add in any bounds from the declaration.
param.bounds.iter().cloned(),
)
.collect();
cx.typaram(param.ident.span.with_ctxt(ctxt), param.ident, bounds, None)
}
GenericParamKind::Const { ty, kw_span, .. } => {
let const_nodefault_kind = GenericParamKind::Const {
ty: ty.clone(),
kw_span: kw_span.with_ctxt(ctxt),
// We can't have default values inside impl block
default: None,
};
let mut param_clone = param.clone();
param_clone.kind = const_nodefault_kind;
param_clone
}

As you've correctly guessed, for type parameters the attributes are effectively removed (well, a new type parameter gets created with cx.typaram which doesn't have any attrs1) and for lifetime and const parameters, they are retained (because the original param gets cloned).

Footnotes

  1. Since 2016: https://github.com/rust-lang/rust/pull/34764/files#diff-c9dd8369ac6e978946e8af0a0730b7ee21eb9791918ec07569ed0f9ea1f0d74cR258

@fmease
Copy link
Member

fmease commented Nov 5, 2024

However, simply removing all attributes also feels very weird to me. It means we strip #[⟨allow|warn|deny|forbid|expect⟩(...)], too. This has the consequence that the following code fails to compile if you uncomment the derive part:

#![deny(non_camel_case_types)]

// #[derive(PartialEq)] // <-- uncomment to make this fail
struct S<#[allow(non_camel_case_types)] t>(t);

@PonasKovas
Copy link
Contributor Author

PonasKovas commented Nov 5, 2024

Yes I agree, maybe there should be a list of built-in attributes that should not be removed? I should probably check how it's done for normal proc macros (not built-in)

If I understand correctly, normal proc macros don't even "see" the helper attributes of other macros. Built-in macros should work the same way, that means removing specifically helper attributes defined by other proc macros on that item

@fmease
Copy link
Member

fmease commented Nov 5, 2024

If I understand correctly, normal proc macros don't even "see" the helper attributes of other macros.

I don't think that is true (but I wish that was case)? How did you test this?

@fmease
Copy link
Member

fmease commented Nov 5, 2024

Yes I agree, maybe there should be a list of built-in attributes that should not be removed?

Well, this all seems a bit tricky. I think you should go ahead and open a PR that removes these attributes unconditionally. I'll think about this some more and probably get into contact with other people to discuss this, too.

@PonasKovas
Copy link
Contributor Author

PonasKovas commented Nov 5, 2024

I don't think that is true (rust-lang/libs-team#334 (comment))? How did you test this?

Oops yeah, you're right.

Well, this all seems a bit tricky. I think you should go ahead and open a PR that removes these attributes unconditionally. I'll think about this some more and probably get into contact with other people to discuss this, too.

Ok sounds good!

@fmease fmease linked a pull request Nov 5, 2024 that will close this issue
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 12, 2024
Remove attributes from generics in built-in derive macros

Related issue rust-lang#132561

Removes all attributes from generics in the expanded implementations of built-in derive macros.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 12, 2024
Rollup merge of rust-lang#132651 - PonasKovas:master, r=fmease

Remove attributes from generics in built-in derive macros

Related issue rust-lang#132561

Removes all attributes from generics in the expanded implementations of built-in derive macros.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-proc-macros Area: Procedural macros C-bug Category: This is a bug. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants