-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Declarative macros: no error about duplicate trait items #71614
Comments
They're seperate items because of hygiene though, right? |
Exactly. This is still a bug though, because the trait has a single item, but the impl has two. |
The names both still "work" for the instance though (in the sense that having one is enough to satisfy the check that the impl covers each item of the trait). When both are present, it silently uses one of them, and I do not know which... |
What should be the correct diagnostic here?
|
The following code should work fine I think: #![feature(decl_macro)]
trait Trait {
type T;
}
macro trait_impl {
() => {
type T = ();
}
}
impl Trait for i32 {
trait_impl!();
} I believe that means only the 2nd error makes sense? |
What about without the trait: should we err? That would be inconsistent with plain items. #![feature(decl_macro)]
macro trait_impl {
() => {
fn foo() {}
}
}
struct Type;
impl Type {
trait_impl!();
fn foo() {}
} |
What do you mean? This errors after "inlining" the macro, right? |
trait_impl!();
fn foo() {} does not error. This is by design, the main difference with Hence the question: should inherent impls follow the "plain function" behaviour, or the trait impl behaviour? |
Ah, so functions inside It would be nice to be able to have macros partially implement a trait though. (We do that in rustc's const-eval.) Conceptually I feel like the names there really refer to the trait, they are not newly declared names but 'resolve' to existing names. Arguably inside the macro it doesn't know which trait to resolve them to so a strict hygiene would entirely forbid this, but also then there'd be no way to around around this, right? |
…rochenkov Check uniqueness of impl items by trait item when applicable. When checking uniqueness of item names in impl blocks, we currently use the same definition of hygiene as for toplevel items. This means that a plain item and one generated by a macro 2.0 do not collide. This hygiene rule does not match with how impl items resolve to associated trait items. As a consequence, we misdiagnose the trait impls. This PR proposes to consider that trait impl items are uses of the corresponding trait items during resolution, instead of checking for duplicates later. An error is emitted when a trait impl item is used twice. There should be no stable breakage, since macros 2.0 are still unstable. r? `@petrochenkov` cc `@RalfJung` Fixes rust-lang#71614.
Check uniqueness of impl items by trait item when applicable. When checking uniqueness of item names in impl blocks, we currently use the same definition of hygiene as for toplevel items. This means that a plain item and one generated by a macro 2.0 do not collide. This hygiene rule does not match with how impl items resolve to associated trait items. As a consequence, we misdiagnose the trait impls. This PR proposes to consider that trait impl items are uses of the corresponding trait items during resolution, instead of checking for duplicates later. An error is emitted when a trait impl item is used twice. There should be no stable breakage, since macros 2.0 are still unstable. r? ``@petrochenkov`` cc ``@RalfJung`` Fixes rust-lang/rust#71614.
Check uniqueness of impl items by trait item when applicable. When checking uniqueness of item names in impl blocks, we currently use the same definition of hygiene as for toplevel items. This means that a plain item and one generated by a macro 2.0 do not collide. This hygiene rule does not match with how impl items resolve to associated trait items. As a consequence, we misdiagnose the trait impls. This PR proposes to consider that trait impl items are uses of the corresponding trait items during resolution, instead of checking for duplicates later. An error is emitted when a trait impl item is used twice. There should be no stable breakage, since macros 2.0 are still unstable. r? ``@petrochenkov`` cc ``@RalfJung`` Fixes rust-lang/rust#71614.
Check uniqueness of impl items by trait item when applicable. When checking uniqueness of item names in impl blocks, we currently use the same definition of hygiene as for toplevel items. This means that a plain item and one generated by a macro 2.0 do not collide. This hygiene rule does not match with how impl items resolve to associated trait items. As a consequence, we misdiagnose the trait impls. This PR proposes to consider that trait impl items are uses of the corresponding trait items during resolution, instead of checking for duplicates later. An error is emitted when a trait impl item is used twice. There should be no stable breakage, since macros 2.0 are still unstable. r? ``@petrochenkov`` cc ``@RalfJung`` Fixes rust-lang/rust#71614.
I tried this code on the latest nightly:
I expected to see this happen: an error about
type T
being defined twice in the same impl. (This is what happens withmacro_rules! trait_impl
.)Instead, this happened: the code just compiles. The same also works when considering a function rather than a type.
The text was updated successfully, but these errors were encountered: