Skip to content

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

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,6 +1118,10 @@ pub trait ResolverExpand {
trait_def_id: DefId,
impl_def_id: LocalDefId,
) -> Result<Vec<(Ident, Option<Ident>)>, Indeterminate>;

/// Record the name of an opaque `Ty::ImplTrait` pre-expansion so that it can be used
/// to generate an item name later that does not reference placeholder macros.
fn insert_impl_trait_name(&mut self, id: NodeId, name: Symbol);
}

pub trait LintStoreExpand {
Expand Down
104 changes: 57 additions & 47 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use rustc_session::lint::builtin::{UNUSED_ATTRIBUTES, UNUSED_DOC_COMMENTS};
use rustc_session::parse::feature_err;
use rustc_session::{Limit, Session};
use rustc_span::hygiene::SyntaxContext;
use rustc_span::{ErrorGuaranteed, FileName, Ident, LocalExpnId, Span, sym};
use rustc_span::{ErrorGuaranteed, FileName, Ident, LocalExpnId, Span, Symbol, sym};
use smallvec::SmallVec;

use crate::base::*;
Expand Down Expand Up @@ -86,7 +86,7 @@ macro_rules! ast_fragments {
}
}

fn make_from<'a>(self, result: Box<dyn MacResult + 'a>) -> Option<AstFragment> {
fn make_from(self, result: Box<dyn MacResult + '_>) -> Option<AstFragment> {
match self {
AstFragmentKind::OptExpr =>
result.make_expr().map(Some).map(AstFragment::OptExpr),
Expand Down Expand Up @@ -136,7 +136,7 @@ macro_rules! ast_fragments {
T::fragment_to_output(self)
}

pub(crate) fn mut_visit_with<F: MutVisitor>(&mut self, vis: &mut F) {
pub(crate) fn mut_visit_with(&mut self, vis: &mut impl MutVisitor) {
match self {
AstFragment::OptExpr(opt_expr) => {
if let Some(expr) = opt_expr.take() {
Expand Down Expand Up @@ -316,9 +316,9 @@ impl AstFragmentKind {
}
}

pub(crate) fn expect_from_annotatables<I: IntoIterator<Item = Annotatable>>(
pub(crate) fn expect_from_annotatables(
self,
items: I,
items: impl IntoIterator<Item = Annotatable>,
) -> AstFragment {
let mut items = items.into_iter();
match self {
Expand Down Expand Up @@ -1218,10 +1218,10 @@ trait InvocationCollectorNode: HasAttrs + HasNodeId + Sized {
fn descr() -> &'static str {
unreachable!()
}
fn walk_flat_map<V: MutVisitor>(self, _visitor: &mut V) -> Self::OutputTy {
fn walk_flat_map(self, _collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
unreachable!()
}
fn walk<V: MutVisitor>(&mut self, _visitor: &mut V) {
fn walk(&mut self, _collector: &mut InvocationCollector<'_, '_>) {
unreachable!()
}
fn is_mac_call(&self) -> bool {
Expand Down Expand Up @@ -1276,8 +1276,8 @@ impl InvocationCollectorNode for P<ast::Item> {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_items()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_item(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_item(collector, self)
}
fn is_mac_call(&self) -> bool {
matches!(self.kind, ItemKind::MacCall(..))
Expand Down Expand Up @@ -1431,8 +1431,8 @@ impl InvocationCollectorNode for AstNodeWrapper<P<ast::AssocItem>, TraitItemTag>
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_trait_items()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_assoc_item(visitor, self.wrapped, AssocCtxt::Trait)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_assoc_item(collector, self.wrapped, AssocCtxt::Trait)
}
fn is_mac_call(&self) -> bool {
matches!(self.wrapped.kind, AssocItemKind::MacCall(..))
Expand Down Expand Up @@ -1472,8 +1472,8 @@ impl InvocationCollectorNode for AstNodeWrapper<P<ast::AssocItem>, ImplItemTag>
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_impl_items()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_assoc_item(visitor, self.wrapped, AssocCtxt::Impl { of_trait: false })
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_assoc_item(collector, self.wrapped, AssocCtxt::Impl { of_trait: false })
}
fn is_mac_call(&self) -> bool {
matches!(self.wrapped.kind, AssocItemKind::MacCall(..))
Expand Down Expand Up @@ -1513,8 +1513,8 @@ impl InvocationCollectorNode for AstNodeWrapper<P<ast::AssocItem>, TraitImplItem
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_trait_impl_items()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_assoc_item(visitor, self.wrapped, AssocCtxt::Impl { of_trait: true })
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_assoc_item(collector, self.wrapped, AssocCtxt::Impl { of_trait: true })
}
fn is_mac_call(&self) -> bool {
matches!(self.wrapped.kind, AssocItemKind::MacCall(..))
Expand Down Expand Up @@ -1551,8 +1551,8 @@ impl InvocationCollectorNode for P<ast::ForeignItem> {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_foreign_items()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_foreign_item(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_foreign_item(collector, self)
}
fn is_mac_call(&self) -> bool {
matches!(self.kind, ForeignItemKind::MacCall(..))
Expand All @@ -1573,8 +1573,8 @@ impl InvocationCollectorNode for ast::Variant {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_variants()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_variant(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_variant(collector, self)
}
}

Expand All @@ -1586,8 +1586,8 @@ impl InvocationCollectorNode for ast::WherePredicate {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_where_predicates()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_where_predicate(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_where_predicate(collector, self)
}
}

Expand All @@ -1599,8 +1599,8 @@ impl InvocationCollectorNode for ast::FieldDef {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_field_defs()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_field_def(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_field_def(collector, self)
}
}

Expand All @@ -1612,8 +1612,8 @@ impl InvocationCollectorNode for ast::PatField {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_pat_fields()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_pat_field(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_pat_field(collector, self)
}
}

Expand All @@ -1625,8 +1625,8 @@ impl InvocationCollectorNode for ast::ExprField {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_expr_fields()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_expr_field(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_expr_field(collector, self)
}
}

Expand All @@ -1638,8 +1638,8 @@ impl InvocationCollectorNode for ast::Param {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_params()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_param(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_param(collector, self)
}
}

Expand All @@ -1651,8 +1651,8 @@ impl InvocationCollectorNode for ast::GenericParam {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_generic_params()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_generic_param(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_generic_param(collector, self)
}
}

Expand All @@ -1664,8 +1664,8 @@ impl InvocationCollectorNode for ast::Arm {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_arms()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_arm(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_arm(collector, self)
}
}

Expand All @@ -1677,8 +1677,8 @@ impl InvocationCollectorNode for ast::Stmt {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_stmts()
}
fn walk_flat_map<V: MutVisitor>(self, visitor: &mut V) -> Self::OutputTy {
walk_flat_map_stmt(visitor, self)
fn walk_flat_map(self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_flat_map_stmt(collector, self)
}
fn is_mac_call(&self) -> bool {
match &self.kind {
Expand Down Expand Up @@ -1751,8 +1751,8 @@ impl InvocationCollectorNode for ast::Crate {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_crate()
}
fn walk<V: MutVisitor>(&mut self, visitor: &mut V) {
walk_crate(visitor, self)
fn walk(&mut self, collector: &mut InvocationCollector<'_, '_>) {
walk_crate(collector, self)
}
fn expand_cfg_false(
&mut self,
Expand All @@ -1777,8 +1777,18 @@ impl InvocationCollectorNode for ast::Ty {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_ty()
}
fn walk<V: MutVisitor>(&mut self, visitor: &mut V) {
walk_ty(visitor, self)
fn walk(&mut self, collector: &mut InvocationCollector<'_, '_>) {
// Save the pre-expanded name of this `ImplTrait`, so that later when defining
// an APIT we use a name that doesn't have any placeholder fragments in it.
if let ast::TyKind::ImplTrait(..) = self.kind {
// HACK: pprust breaks strings with newlines when the type
// gets too long. We don't want these to show up in compiler
// output or built artifacts, so replace them here...
// Perhaps we should instead format APITs more robustly.
let name = Symbol::intern(&pprust::ty_to_string(self).replace('\n', " "));
collector.cx.resolver.insert_impl_trait_name(self.id, name);
}
walk_ty(collector, self)
}
fn is_mac_call(&self) -> bool {
matches!(self.kind, ast::TyKind::MacCall(..))
Expand All @@ -1800,8 +1810,8 @@ impl InvocationCollectorNode for ast::Pat {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_pat()
}
fn walk<V: MutVisitor>(&mut self, visitor: &mut V) {
walk_pat(visitor, self)
fn walk(&mut self, collector: &mut InvocationCollector<'_, '_>) {
walk_pat(collector, self)
}
fn is_mac_call(&self) -> bool {
matches!(self.kind, PatKind::MacCall(..))
Expand All @@ -1826,8 +1836,8 @@ impl InvocationCollectorNode for ast::Expr {
fn descr() -> &'static str {
"an expression"
}
fn walk<V: MutVisitor>(&mut self, visitor: &mut V) {
walk_expr(visitor, self)
fn walk(&mut self, collector: &mut InvocationCollector<'_, '_>) {
walk_expr(collector, self)
}
fn is_mac_call(&self) -> bool {
matches!(self.kind, ExprKind::MacCall(..))
Expand All @@ -1850,8 +1860,8 @@ impl InvocationCollectorNode for AstNodeWrapper<P<ast::Expr>, OptExprTag> {
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
fragment.make_opt_expr()
}
fn walk_flat_map<V: MutVisitor>(mut self, visitor: &mut V) -> Self::OutputTy {
walk_expr(visitor, &mut self.wrapped);
fn walk_flat_map(mut self, collector: &mut InvocationCollector<'_, '_>) -> Self::OutputTy {
walk_expr(collector, &mut self.wrapped);
Some(self.wrapped)
}
fn is_mac_call(&self) -> bool {
Expand Down Expand Up @@ -1885,8 +1895,8 @@ impl InvocationCollectorNode for AstNodeWrapper<P<ast::Expr>, MethodReceiverTag>
fn fragment_to_output(fragment: AstFragment) -> Self::OutputTy {
AstNodeWrapper::new(fragment.make_method_receiver_expr(), MethodReceiverTag)
}
fn walk<V: MutVisitor>(&mut self, visitor: &mut V) {
walk_expr(visitor, &mut self.wrapped)
fn walk(&mut self, collector: &mut InvocationCollector<'_, '_>) {
walk_expr(collector, &mut self.wrapped)
}
fn is_mac_call(&self) -> bool {
matches!(self.wrapped.kind, ast::ExprKind::MacCall(..))
Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_resolve/src/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ use std::mem;

use rustc_ast::visit::FnKind;
use rustc_ast::*;
use rustc_ast_pretty::pprust;
use rustc_attr_parsing::{AttributeParser, OmitDoc};
use rustc_expand::expand::AstFragment;
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, CtorOf, DefKind};
use rustc_hir::def_id::LocalDefId;
use rustc_middle::span_bug;
use rustc_span::hygiene::LocalExpnId;
use rustc_span::{Span, Symbol, sym};
use tracing::debug;
Expand Down Expand Up @@ -380,20 +380,20 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> {
}

fn visit_ty(&mut self, ty: &'a Ty) {
match &ty.kind {
match ty.kind {
TyKind::MacCall(..) => self.visit_macro_invoc(ty.id),
TyKind::ImplTrait(id, _) => {
// HACK: pprust breaks strings with newlines when the type
// gets too long. We don't want these to show up in compiler
// output or built artifacts, so replace them here...
// Perhaps we should instead format APITs more robustly.
let name = Symbol::intern(&pprust::ty_to_string(ty).replace('\n', " "));
TyKind::ImplTrait(opaque_id, _) => {
let name = *self
.resolver
.impl_trait_names
.get(&ty.id)
.unwrap_or_else(|| span_bug!(ty.span, "expected this opaque to be named"));
let kind = match self.invocation_parent.impl_trait_context {
ImplTraitContext::Universal => DefKind::TyParam,
ImplTraitContext::Existential => DefKind::OpaqueTy,
ImplTraitContext::InBinding => return visit::walk_ty(self, ty),
};
let id = self.create_def(*id, Some(name), kind, ty.span);
let id = self.create_def(opaque_id, Some(name), kind, ty.span);
match self.invocation_parent.impl_trait_context {
// Do not nest APIT, as we desugar them as `impl_trait: bounds`,
// so the `impl_trait` node is not a parent to `bounds`.
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,11 @@ pub struct Resolver<'ra, 'tcx> {
current_crate_outer_attr_insert_span: Span,

mods_with_parse_errors: FxHashSet<DefId>,

// Stores pre-expansion and pre-placeholder-fragment-insertion names for `impl Trait` types
// that were encountered during resolution. These names are used to generate item names
// for APITs, so we don't want to leak details of resolution into these names.
impl_trait_names: FxHashMap<NodeId, Symbol>,
}

/// This provides memory for the rest of the crate. The `'ra` lifetime that is
Expand Down Expand Up @@ -1579,6 +1584,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
impl_binding_keys: Default::default(),
current_crate_outer_attr_insert_span,
mods_with_parse_errors: Default::default(),
impl_trait_names: Default::default(),
};

let root_parent_scope = ParentScope::module(graph_root, &resolver);
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,10 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
});
Ok(idents)
}

fn insert_impl_trait_name(&mut self, id: NodeId, name: Symbol) {
self.impl_trait_names.insert(id, name);
}
}

impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
Expand Down
9 changes: 0 additions & 9 deletions tests/crashes/140333.rs

This file was deleted.

12 changes: 12 additions & 0 deletions tests/ui/impl-trait/name-mentioning-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
trait Foo<T> {}

macro_rules! bar {
() => { () }
}

fn foo(x: impl Foo<bar!()>) {
let () = x;
//~^ ERROR mismatched types
}

fn main() {}
Loading
Loading