From 64fd8980f6c292b17096f16a12f3a3b0a3727318 Mon Sep 17 00:00:00 2001 From: Boxy Date: Thu, 21 Nov 2024 10:19:40 +0000 Subject: [PATCH] Create anon const defs in `DefCollector` unconditionally --- compiler/rustc_ast/src/ast.rs | 27 --- compiler/rustc_ast_lowering/src/asm.rs | 17 +- compiler/rustc_ast_lowering/src/expr.rs | 8 +- compiler/rustc_ast_lowering/src/lib.rs | 18 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 13 ++ compiler/rustc_resolve/src/def_collector.rs | 156 +++--------------- compiler/rustc_resolve/src/lib.rs | 13 -- .../const-arg-empty-macro-expansion-2.rs | 18 ++ .../const-arg-empty-macro-expansion-2.stderr | 54 ++++++ .../early/const-arg-empty-macro-expansion.rs | 14 ++ .../const-arg-empty-macro-expansion.stderr | 15 ++ tests/ui/consts/issue-36163.stderr | 6 +- 12 files changed, 154 insertions(+), 205 deletions(-) create mode 100644 tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.rs create mode 100644 tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.stderr create mode 100644 tests/ui/const-generics/early/const-arg-empty-macro-expansion.rs create mode 100644 tests/ui/const-generics/early/const-arg-empty-macro-expansion.stderr diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 5f71fb97d768c..2b35f17795345 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1217,33 +1217,6 @@ impl Expr { } } - /// Determines whether this expression is a macro call optionally wrapped in braces . If - /// `already_stripped_block` is set then we do not attempt to peel off a layer of braces. - /// - /// Returns the [`NodeId`] of the macro call and whether a layer of braces has been peeled - /// either before, or part of, this function. - pub fn optionally_braced_mac_call( - &self, - already_stripped_block: bool, - ) -> Option<(bool, NodeId)> { - match &self.kind { - ExprKind::Block(block, None) - if let [stmt] = &*block.stmts - && !already_stripped_block => - { - match &stmt.kind { - StmtKind::MacCall(_) => Some((true, stmt.id)), - StmtKind::Expr(expr) if let ExprKind::MacCall(_) = &expr.kind => { - Some((true, expr.id)) - } - _ => None, - } - } - ExprKind::MacCall(_) => Some((already_stripped_block, self.id)), - _ => None, - } - } - pub fn to_bound(&self) -> Option { match &self.kind { ExprKind::Path(None, path) => Some(GenericBound::Trait(PolyTraitRef::new( diff --git a/compiler/rustc_ast_lowering/src/asm.rs b/compiler/rustc_ast_lowering/src/asm.rs index 215e6d84d0f0a..48cc6a29560e5 100644 --- a/compiler/rustc_ast_lowering/src/asm.rs +++ b/compiler/rustc_ast_lowering/src/asm.rs @@ -222,16 +222,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // Wrap the expression in an AnonConst. let parent_def_id = self.current_def_id_parent; let node_id = self.next_node_id(); - // HACK(min_generic_const_args): see lower_anon_const - if !expr.is_potential_trivial_const_arg(true) { - self.create_def( - parent_def_id, - node_id, - kw::Empty, - DefKind::AnonConst, - *op_sp, - ); - } + self.create_def( + parent_def_id, + node_id, + kw::Empty, + DefKind::AnonConst, + *op_sp, + ); let anon_const = AnonConst { id: node_id, value: P(expr) }; hir::InlineAsmOperand::SymFn { anon_const: self.lower_anon_const_to_anon_const(&anon_const), diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 3af29838b72c4..02f8d01bab83f 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -455,11 +455,9 @@ impl<'hir> LoweringContext<'_, 'hir> { let parent_def_id = self.current_def_id_parent; let node_id = self.next_node_id(); - // HACK(min_generic_const_args): see lower_anon_const - if !arg.is_potential_trivial_const_arg(true) { - // Add a definition for the in-band const def. - self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span); - } + // Add a definition for the const argument as it was not created by the def collector as we + // require name resolution results in order to even know this was a const argument. + self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span); let mut visitor = WillCreateDefIdsVisitor {}; let const_value = if let ControlFlow::Break(span) = visitor.visit_expr(&arg) { diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 0b2969a49ba88..cd9ebfcebd37d 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -2053,8 +2053,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } /// Used when lowering a type argument that turned out to actually be a const argument. - /// - /// Only use for that purpose since otherwise it will create a duplicate def. #[instrument(level = "debug", skip(self))] fn lower_const_path_to_const_arg( &mut self, @@ -2164,7 +2162,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ); return ConstArg { - hir_id: self.next_id(), + hir_id: self.lower_node_id(anon.id), kind: hir::ConstArgKind::Path(qpath), is_desugared_from_effects: false, }; @@ -2181,20 +2179,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { /// See [`hir::ConstArg`] for when to use this function vs /// [`Self::lower_anon_const_to_const_arg`]. fn lower_anon_const_to_anon_const(&mut self, c: &AnonConst) -> &'hir hir::AnonConst { - if c.value.is_potential_trivial_const_arg(true) { - // HACK(min_generic_const_args): see DefCollector::visit_anon_const - // Over there, we guess if this is a bare param and only create a def if - // we think it's not. However we may can guess wrong (see there for example) - // in which case we have to create the def here. - self.create_def( - self.current_def_id_parent, - c.id, - kw::Empty, - DefKind::AnonConst, - c.value.span, - ); - } - self.arena.alloc(self.with_new_scopes(c.value.span, |this| { let def_id = this.local_def_id(c.id); let hir_id = this.lower_node_id(c.id); diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index b5391247cea54..44e9148e8d832 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1383,6 +1383,19 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let def_id = local_id.to_def_id(); let def_kind = tcx.def_kind(local_id); self.tables.def_kind.set_some(def_id.index, def_kind); + + // The `DefCollector` will sometimes hallucinate unnecessary `DefId`s for const arguments + // that do not actually correspond to anything. Avoid encoding anything for these `DefId`s + // as lots of queries do not support being called with these hallucinated `DefId`s. + if def_kind == DefKind::AnonConst + && matches!( + tcx.hir_node_by_def_id(local_id), + hir::Node::ConstArg(hir::ConstArg { kind: hir::ConstArgKind::Path(..), .. }) + ) + { + continue; + } + if should_encode_span(def_kind) { let def_span = tcx.def_span(local_id); record!(self.tables.def_span[def_id] <- def_span); diff --git a/compiler/rustc_resolve/src/def_collector.rs b/compiler/rustc_resolve/src/def_collector.rs index bf27b767a4972..a7ca057c2d988 100644 --- a/compiler/rustc_resolve/src/def_collector.rs +++ b/compiler/rustc_resolve/src/def_collector.rs @@ -12,23 +12,16 @@ use rustc_span::hygiene::LocalExpnId; use rustc_span::symbol::{Symbol, kw, sym}; use tracing::debug; -use crate::{ImplTraitContext, InvocationParent, PendingAnonConstInfo, Resolver}; +use crate::{ImplTraitContext, InvocationParent, Resolver}; pub(crate) fn collect_definitions( resolver: &mut Resolver<'_, '_>, fragment: &AstFragment, expansion: LocalExpnId, ) { - let InvocationParent { parent_def, pending_anon_const_info, impl_trait_context, in_attr } = + let InvocationParent { parent_def, impl_trait_context, in_attr } = resolver.invocation_parents[&expansion]; - let mut visitor = DefCollector { - resolver, - parent_def, - pending_anon_const_info, - expansion, - impl_trait_context, - in_attr, - }; + let mut visitor = DefCollector { resolver, parent_def, expansion, impl_trait_context, in_attr }; fragment.visit_with(&mut visitor); } @@ -36,13 +29,6 @@ pub(crate) fn collect_definitions( struct DefCollector<'a, 'ra, 'tcx> { resolver: &'a mut Resolver<'ra, 'tcx>, parent_def: LocalDefId, - /// If we have an anon const that consists of a macro invocation, e.g. `Foo<{ m!() }>`, - /// we need to wait until we know what the macro expands to before we create the def for - /// the anon const. That's because we lower some anon consts into `hir::ConstArgKind::Path`, - /// which don't have defs. - /// - /// See `Self::visit_anon_const()`. - pending_anon_const_info: Option, impl_trait_context: ImplTraitContext, in_attr: bool, expansion: LocalExpnId, @@ -110,61 +96,13 @@ impl<'a, 'ra, 'tcx> DefCollector<'a, 'ra, 'tcx> { fn visit_macro_invoc(&mut self, id: NodeId) { let id = id.placeholder_to_expn_id(); - let pending_anon_const_info = self.pending_anon_const_info.take(); let old_parent = self.resolver.invocation_parents.insert(id, InvocationParent { parent_def: self.parent_def, - pending_anon_const_info, impl_trait_context: self.impl_trait_context, in_attr: self.in_attr, }); assert!(old_parent.is_none(), "parent `LocalDefId` is reset for an invocation"); } - - /// Determines whether the const argument `AnonConst` is a simple macro call, optionally - /// surrounded with braces. - /// - /// If this const argument *is* a trivial macro call then the id for the macro call is - /// returned along with the information required to build the anon const's def if - /// the macro call expands to a non-trivial expression. - fn is_const_arg_trivial_macro_expansion( - &self, - anon_const: &'a AnonConst, - ) -> Option<(PendingAnonConstInfo, NodeId)> { - anon_const.value.optionally_braced_mac_call(false).map(|(block_was_stripped, id)| { - ( - PendingAnonConstInfo { - id: anon_const.id, - span: anon_const.value.span, - block_was_stripped, - }, - id, - ) - }) - } - - /// Determines whether the expression `const_arg_sub_expr` is a simple macro call, sometimes - /// surrounded with braces if a set of braces has not already been entered. This is required - /// as `{ N }` is treated as equivalent to a bare parameter `N` whereas `{{ N }}` is treated as - /// a real block expression and is lowered to an anonymous constant which is not allowed to use - /// generic parameters. - /// - /// If this expression is a trivial macro call then the id for the macro call is - /// returned along with the information required to build the anon const's def if - /// the macro call expands to a non-trivial expression. - fn is_const_arg_sub_expr_trivial_macro_expansion( - &self, - const_arg_sub_expr: &'a Expr, - ) -> Option<(PendingAnonConstInfo, NodeId)> { - let pending_anon = self.pending_anon_const_info.unwrap_or_else(|| - panic!("Checking expr is trivial macro call without having entered anon const: `{const_arg_sub_expr:?}`"), - ); - - const_arg_sub_expr.optionally_braced_mac_call(pending_anon.block_was_stripped).map( - |(block_was_stripped, id)| { - (PendingAnonConstInfo { block_was_stripped, ..pending_anon }, id) - }, - ) - } } impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> { @@ -376,78 +314,36 @@ impl<'a, 'ra, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'ra, 'tcx> { } fn visit_anon_const(&mut self, constant: &'a AnonConst) { - // HACK(min_generic_const_args): don't create defs for anon consts if we think they will - // later be turned into ConstArgKind::Path's. because this is before resolve is done, we - // may accidentally identify a construction of a unit struct as a param and not create a - // def. we'll then create a def later in ast lowering in this case. the parent of nested - // items will be messed up, but that's ok because there can't be any if we're just looking - // for bare idents. - - if let Some((pending_anon, macro_invoc)) = - self.is_const_arg_trivial_macro_expansion(constant) - { - self.pending_anon_const_info = Some(pending_anon); - return self.visit_macro_invoc(macro_invoc); - } else if constant.value.is_potential_trivial_const_arg(true) { - return visit::walk_anon_const(self, constant); - } - + // Not all anon consts should actually have a `DefId` created for them as sometimes we lower const arguments + // to `ConstArgKind::Path` rather than an anonymous const. Unfortuantely handling that correctly in the presence + // of macros is not possible so we sometimes hallucinate `DefId`s here. let def = self.create_def(constant.id, kw::Empty, DefKind::AnonConst, constant.value.span); self.with_parent(def, |this| visit::walk_anon_const(this, constant)); } fn visit_expr(&mut self, expr: &'a Expr) { - // If we're visiting the expression of a const argument that was a macro call then - // check if it is *still* unknown whether it is a trivial const arg or not. If so - // recurse into the macro call and delay creating the anon const def until expansion. - if self.pending_anon_const_info.is_some() - && let Some((pending_anon, macro_invoc)) = - self.is_const_arg_sub_expr_trivial_macro_expansion(expr) - { - self.pending_anon_const_info = Some(pending_anon); - return self.visit_macro_invoc(macro_invoc); - } - - // See self.pending_anon_const_info for explanation - let parent_def = self - .pending_anon_const_info - .take() - // If we already stripped away a set of braces then do not do it again when determining - // if the macro expanded to a trivial const arg. This arises in cases such as: - // `Foo<{ bar!() }>` where `bar!()` expands to `{ N }`. This should not be considered a - // trivial const argument even though `{ N }` by itself *is*. - .filter(|pending_anon| { - !expr.is_potential_trivial_const_arg(!pending_anon.block_was_stripped) - }) - .map(|pending_anon| { - self.create_def(pending_anon.id, kw::Empty, DefKind::AnonConst, pending_anon.span) - }) - .unwrap_or(self.parent_def); - - self.with_parent(parent_def, |this| { - let parent_def = match expr.kind { - ExprKind::MacCall(..) => return this.visit_macro_invoc(expr.id), - ExprKind::Closure(..) | ExprKind::Gen(..) => { - this.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span) - } - ExprKind::ConstBlock(ref constant) => { - for attr in &expr.attrs { - visit::walk_attribute(this, attr); - } - let def = this.create_def( - constant.id, - kw::Empty, - DefKind::InlineConst, - constant.value.span, - ); - this.with_parent(def, |this| visit::walk_anon_const(this, constant)); - return; + let parent_def = match expr.kind { + ExprKind::MacCall(..) => return self.visit_macro_invoc(expr.id), + ExprKind::Closure(..) | ExprKind::Gen(..) => { + self.create_def(expr.id, kw::Empty, DefKind::Closure, expr.span) + } + ExprKind::ConstBlock(ref constant) => { + for attr in &expr.attrs { + visit::walk_attribute(self, attr); } - _ => this.parent_def, - }; + let def = self.create_def( + constant.id, + kw::Empty, + DefKind::InlineConst, + constant.value.span, + ); + self.with_parent(def, |this| visit::walk_anon_const(this, constant)); + return; + } + _ => self.parent_def, + }; - this.with_parent(parent_def, |this| visit::walk_expr(this, expr)) - }) + self.with_parent(parent_def, |this| visit::walk_expr(this, expr)) } fn visit_ty(&mut self, ty: &'a Ty) { diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index e382295b8f6d8..094871ad26876 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -174,7 +174,6 @@ impl<'ra> ParentScope<'ra> { #[derive(Copy, Debug, Clone)] struct InvocationParent { parent_def: LocalDefId, - pending_anon_const_info: Option, impl_trait_context: ImplTraitContext, in_attr: bool, } @@ -182,23 +181,11 @@ struct InvocationParent { impl InvocationParent { const ROOT: Self = Self { parent_def: CRATE_DEF_ID, - pending_anon_const_info: None, impl_trait_context: ImplTraitContext::Existential, in_attr: false, }; } -#[derive(Copy, Debug, Clone)] -struct PendingAnonConstInfo { - // A const arg is only a "trivial" const arg if it has at *most* one set of braces - // around the argument. We track whether we have stripped an outter brace so that - // if a macro expands to a braced expression *and* the macro was itself inside of - // some braces then we can consider it to be a non-trivial const argument. - block_was_stripped: bool, - id: NodeId, - span: Span, -} - #[derive(Copy, Debug, Clone)] enum ImplTraitContext { Existential, diff --git a/tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.rs b/tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.rs new file mode 100644 index 0000000000000..a822ec3423e06 --- /dev/null +++ b/tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.rs @@ -0,0 +1,18 @@ +impl + Foo< + //~^ ERROR: cannot find type `Foo` in this scope + T, + { + thread_local! { pub static FOO : Foo = Foo { } ; } + //~^ ERROR: cannot find type `Foo` in this scope + //~| ERROR: cannot find type `Foo` in this scope + //~| ERROR: cannot find type `Foo` in this scope + //~| ERROR: cannot find type `Foo` in this scope + //~| ERROR: cannot find type `Foo` in this scope + //~| ERROR: cannot find struct, variant or union type `Foo` in this scope + }, + > +{ +} + +fn main() {} diff --git a/tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.stderr b/tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.stderr new file mode 100644 index 0000000000000..8e2767c342251 --- /dev/null +++ b/tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.stderr @@ -0,0 +1,54 @@ +error[E0412]: cannot find type `Foo` in this scope + --> $DIR/const-arg-empty-macro-expansion-2.rs:2:5 + | +LL | Foo< + | ^^^ not found in this scope + +error[E0412]: cannot find type `Foo` in this scope + --> $DIR/const-arg-empty-macro-expansion-2.rs:6:46 + | +LL | thread_local! { pub static FOO : Foo = Foo { } ; } + | ^^^ not found in this scope + +error[E0412]: cannot find type `Foo` in this scope + --> $DIR/const-arg-empty-macro-expansion-2.rs:6:46 + | +LL | thread_local! { pub static FOO : Foo = Foo { } ; } + | ^^^ not found in this scope + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0422]: cannot find struct, variant or union type `Foo` in this scope + --> $DIR/const-arg-empty-macro-expansion-2.rs:6:52 + | +LL | thread_local! { pub static FOO : Foo = Foo { } ; } + | ^^^ not found in this scope + +error[E0412]: cannot find type `Foo` in this scope + --> $DIR/const-arg-empty-macro-expansion-2.rs:6:46 + | +LL | thread_local! { pub static FOO : Foo = Foo { } ; } + | ^^^ not found in this scope + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0412]: cannot find type `Foo` in this scope + --> $DIR/const-arg-empty-macro-expansion-2.rs:6:46 + | +LL | thread_local! { pub static FOO : Foo = Foo { } ; } + | ^^^ not found in this scope + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0412]: cannot find type `Foo` in this scope + --> $DIR/const-arg-empty-macro-expansion-2.rs:6:46 + | +LL | thread_local! { pub static FOO : Foo = Foo { } ; } + | ^^^ not found in this scope + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: aborting due to 7 previous errors + +Some errors have detailed explanations: E0412, E0422. +For more information about an error, try `rustc --explain E0412`. diff --git a/tests/ui/const-generics/early/const-arg-empty-macro-expansion.rs b/tests/ui/const-generics/early/const-arg-empty-macro-expansion.rs new file mode 100644 index 0000000000000..eb0f76409dd0b --- /dev/null +++ b/tests/ui/const-generics/early/const-arg-empty-macro-expansion.rs @@ -0,0 +1,14 @@ +macro_rules! empty { + () => {}; +} + +struct Foo; + +#[rustfmt::skip] +impl Foo<{ empty!{}; }> {} +//~^ ERROR: mismatched types +#[rustfmt::skip] +impl Foo<{ empty!(); }> {} +//~^ ERROR: mismatched types + +fn main() {} diff --git a/tests/ui/const-generics/early/const-arg-empty-macro-expansion.stderr b/tests/ui/const-generics/early/const-arg-empty-macro-expansion.stderr new file mode 100644 index 0000000000000..6872393167669 --- /dev/null +++ b/tests/ui/const-generics/early/const-arg-empty-macro-expansion.stderr @@ -0,0 +1,15 @@ +error[E0308]: mismatched types + --> $DIR/const-arg-empty-macro-expansion.rs:8:10 + | +LL | impl Foo<{ empty!{}; }> {} + | ^^^^^^^^^^^^^ expected `usize`, found `()` + +error[E0308]: mismatched types + --> $DIR/const-arg-empty-macro-expansion.rs:11:10 + | +LL | impl Foo<{ empty!(); }> {} + | ^^^^^^^^^^^^^ expected `usize`, found `()` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/consts/issue-36163.stderr b/tests/ui/consts/issue-36163.stderr index 52d3e003f0ac9..8a7a0981f4154 100644 --- a/tests/ui/consts/issue-36163.stderr +++ b/tests/ui/consts/issue-36163.stderr @@ -1,10 +1,10 @@ -error[E0391]: cycle detected when simplifying constant for the type system `Foo::{constant#0}` +error[E0391]: cycle detected when simplifying constant for the type system `Foo::B::{constant#0}` --> $DIR/issue-36163.rs:4:9 | LL | B = A, | ^ | -note: ...which requires const-evaluating + checking `Foo::{constant#0}`... +note: ...which requires const-evaluating + checking `Foo::B::{constant#0}`... --> $DIR/issue-36163.rs:4:9 | LL | B = A, @@ -19,7 +19,7 @@ note: ...which requires const-evaluating + checking `A`... | LL | const A: isize = Foo::B as isize; | ^^^^^^^^^^^^^^^ - = note: ...which again requires simplifying constant for the type system `Foo::{constant#0}`, completing the cycle + = note: ...which again requires simplifying constant for the type system `Foo::B::{constant#0}`, completing the cycle note: cycle used when checking that `Foo` is well-formed --> $DIR/issue-36163.rs:3:1 |