Don't give APITs names with macro expansion placeholder fragments in it

[compiler-errors]

The DefCollector previously called pprust::ty_to_string to construct a name for APITs (arg-position impl traits). The ast::Ty that was being formatted however has already had its macro calls replaced with "placeholder fragments", which end up rendering like !() (or ICEing, in the case of #140333, since it led to a placeholder struct field with no name).

Instead, collect the name of the APIT before we visit its macros and replace them with placeholders in the macro expander. This makes the implementation a bit more involved, but AFAICT there's no better way to do this since we can't do a reverse mapping from placeholder fragment -> original macro call AST.

Fixes #140333

[rustbot]

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

[compiler-errors]

For contrast, this used to render like impl Foo<!()>, where !() is the placeholder fragment.

[davidtwco]

r=me if @petrochenkov is happy since he's also assigned

[petrochenkov]

I dislike this part in particular.

Perhaps the insert_impl_trait_name call can be moved to InvocationCollectorNode::walk on types?
In that case the node ID will already be assigned.
It will be also be a more correct location to do this in general (e.g. compatible with supporting #[cfg] and macro attributes on types).

[compiler-errors]

If I move it to InvocationCollectorNode::walk, then all I have access to is a MutVisitor, so I can't actually call something like insert_impl_trait_name.

[compiler-errors]

flipping state back b/c of a pending question

@rustbot ready

[bors]

☔ The latest upstream changes (presumably #142644) made this pull request unmergeable. Please resolve the merge conflicts.

[petrochenkov]

#142690 should help with #142393 (comment)
(Also node id of ast::Ty itself will need to be used as a key, to avoid the additional node id assignment reordering.)
@rustbot blocked

[compiler-errors]

(Also node id of ast::Ty itself will need to be used as a key, to avoid the additional node id assignment reordering.)

Yeah, I don't think it matters which node id was being used as a key. I think that PR + using the ty's node id is sufficient.

[bors]

☔ The latest upstream changes (presumably #142795) made this pull request unmergeable. Please resolve the merge conflicts.