enable macro visits in deriving

[durka]

Fixes #32950.

There's a warning in src/libsyntax/visit.rs saying not to do this, but it seems to fix the ICE and the other deriving tests survive. The "note about macros above" seems to be this, but deriving is just kinda pasting stuff around, like a macro, so it seems okay.

cc #27245

[rust-highfive]

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

[durka]

There's a subtle issue here. The reason derivings walk the member types is to find mentions of associated types, to add bounds. If a type macro generates this mention, then with this patch the deriving code will miss it:

macro_rules! useassoc {
    ($t:ident, $assoc:ident) => { ($t, $t::$assoc) }
}

trait HasAssoc { type Type; }

#[derive(Debug)]
struct Thing1<T: HasAssoc> {
    thing: useassoc!(T, Type)
}
#[derive(Debug)]
struct Thing2<T: HasAssoc> {
    thing: (T, T::Type)
}

generates

impl <T: ::std::fmt::Debug + HasAssoc> ::std::fmt::Debug for Thing1<T> {
    fn fmt(&self, __arg_0: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        match *self {
            Thing1 { thing: ref __self_0_0 } => {
                let mut builder = __arg_0.debug_struct("Thing1");
                let _ = builder.field("thing", &&(*__self_0_0));
                builder.finish()
            }
        }
    }
}

impl <T: ::std::fmt::Debug + HasAssoc> ::std::fmt::Debug for Thing2<T> where
 T::Type: ::std::fmt::Debug {
    fn fmt(&self, __arg_0: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        match *self {
            Thing2 { thing: ref __self_0_0 } => {
                let mut builder = __arg_0.debug_struct("Thing2");
                let _ = builder.field("thing", &&(*__self_0_0));
                builder.finish()
            }
        }
    }
}

and there we've got a Debug impl that will fail typeck 😿

Fully fixing this seems like it would require moving derivings to after macro expansion, which sounds unlikely. A possible hack is to just copy the macro and generate where useassoc!(T, Type): Debug, but that can fail due to privacy unless derived impls grow special permissions to ignore privates-in-public. Lastly, a conservative fix would be to just remove the ICE and trigger an error saying "please don't use type macros with deriving".

[durka]

Pushed a new test which will fail Travis, to show the problem.

[aturon]

r? @nrc

[nrc]

Overriding visit_mac is fine, it's more of a precaution that you should be aware of the difference between pre- and post-expansion ASTs.

[nrc]

Expanding derive happens at the same time as macro expansion. I expect that the right fix for this is that if derive encounters a macro in type position, then to eagerly expand it. That should be possible, because the input to derive must parse (we have the AST, not just tokens). Changing the ordering of expansion in this case should be OK, because we know exactly what derive is doing, and this should just be a change to the derive code, not a change to the expander. However, I can imagine this might be easier said than done, in which case filing an issue for the non-type checking expansion and leaving a FIXME in the code seem fine to me.

[durka]

@nrc any pointers for functions I can call to expand the macro? Failing that, am I reading you correctly to say that having #[derive] generate an impl with a missing bound on the associated type is preferable to throwing an error saying "type macros not yet supported in #[derive]"?

[durka]

Per discussion on IRC, there are four options:

1. Change #[derive] to emit an error if it sees a type macro, leaving all future options open for how/whether to support them.

2. When #[derive] sees a type macro, copy it to the where clause of the generated impl. This may produce an impl that does not typecheck, if the macro expansion contains private types (this is why #[derive] doesn't simply copy all field types to the where clause). If type macros become stable, this implies a stability guarantee that type macros can be used with #[derive] as long as they don't expand to public types in a private interface. @eddyb wants to write an RFC to elaborate types in bounds that would make this privacy business irrelevant.

   2a. Emit a warning about possibly ill-typed impls when this happens, to assist people in finding the cause of the error.

3. When #[derive] encounters a type macro, eagerly expand it. This seems difficult and could complicate future changes to macro expansion.

4. Do nothing until @eddyb finishes his RFC for elaborating types in bounds.

My preference is for 3 if possible, then 2a, and then 1. Other opinions?

[durka]

Closing as #32950 was fixed by #34010.

[jseyfried]

@durka want to reopen and land this?
We're going to revert my fix so that MultiDecorators can be a special case of MultiModifiers as discussed in IRC.

[durka]

Sure. I'm out of town until Monday though.
On Jun 23, 2016 21:09, "Jeffrey Seyfried" notifications@github.com wrote:

@durka https://github.com/durka want to reopen and land this?
We're going to revert my fix (#34010
#34010) so that MultiDecorators
can be a special case of MultiModifiers as discussed in IRC.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#33769 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAC3n5-ATHW5-JZtr2q5vcDhexOjCP6eks5qOy48gaJpZM4IjtGH
.

[jseyfried]

@durka ok, I might cherry-pick a commit off this branch then.

[eddyb]

@durka @nikomatsakis Just opened rust-lang/rfcs#1671 which covers the deriving privacy interactions.