Implement PartialEq for Ident, PartialEq for TokenStream

[dtolnay]

The original hesitation around these impls was whether to consider things equal that have different Spans.

I propose that if somebody is writing a macro that relies on knowing whether two tokens are the very same down to their syntax context, we should consider that an unhelpable case of "doing it wrong." On the other hand comparing the text of an Ident is quite common and perfectly correct. For example when iterating through field attributes in a custom derive to handle something like #[serde(rename = "...")] it makes sense to compare whether the idents in the token stream are == i_serde or == i_rename.

tracking issue: #38356
cc @Eijebong @alexcrichton

[alexcrichton]

I'm curious to hear @nrc's thoughts on this, but I'd personally prefer if we don't do this quite just yet. Hygeiene is enough of an open question and these seem like they'll be a small enough quality of life improvement that I'd prefer to take the conservative route until we have more information on the hygiene story.

Now that being said, if @nrc isn't necessarily diametrically opposed to this I think we can probably go ahead and do this in proc-macro2 easily and that'd defacto stabilize such behavior for the ecosystem anyway, but we'd still have the option in our back pocket for diverging the official proc_macro's behavior at some point (and a breaking change to proc-macro2)

[dtolnay]

Seems reasonable, unscheduling from 1.2.

[petrochenkov]

For the compiler's internal versions of Ident and TokenStream I plan to remove these impls because "comparison in vacuum" doesn't generally make sense in presence of hygiene and it became a source of bugs.

Normally you want to strip some layers of macro expansions from the ident's context before comparing - "legacy" macro_rules! layers, only "transparent" layers for identifiers with call-site hygiene (when #50050 (comment) is implemented), all layers for string-based comparison, sometimes nothing for lints.

If macro authors want to do string-based comparisons, this should preferably be done in some explicit way.

[nrc]

"comparison in vacuum" doesn't generally make sense in presence of hygiene and it became a source of bugs. [...] If macro authors want to do string-based comparisons, this should preferably be done in some explicit way.

I totally agree - if you have two idents from different hygiene contexts, I don't think you'd ever want them to compare equal (in fact, for some hygiene algorithms, equality as a concept doesn't make sense at all).

[alexcrichton]

Ok given that proc-macro2 is sort of directly targeted at macros 1.2 it sounds like it wouldn't be the worst idea to have these impls in proc-macro2, and by the time that macros 2.0 rolls around things may be different enough that there's a clear path forward?

In any case proc-macro2 is purely an ecosystem thing today, so it shouldn't affect the compiler too much!

[dtolnay]

@petrochenkov would you consider an Ident—string implementation of PartialEq?

impl PartialEq<str> for Ident
impl<'a> PartialEq<&'a str> for Ident

These would address the use case of checking for keywords / recognized attribute names while being unmistakable in terms of how they compare hygiene.

[petrochenkov]

@dtolnay
Yeah, that's probably explicit enough.
(It also looks like proc-macro2 can provide these impls even without proc-macro support.)

[steveklabnik]

Triage: these types do not implement these traits today.