Skip to content
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

Fix anon const def-creation when macros are involved #129137

Merged
merged 2 commits into from
Sep 13, 2024
Merged
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
20 changes: 12 additions & 8 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1188,14 +1188,7 @@ impl Expr {
///
/// Does not ensure that the path resolves to a const param, the caller should check this.
pub fn is_potential_trivial_const_arg(&self) -> bool {
let this = if let ExprKind::Block(block, None) = &self.kind
&& let [stmt] = block.stmts.as_slice()
&& let StmtKind::Expr(expr) = &stmt.kind
{
expr
} else {
self
};
let this = self.maybe_unwrap_block();

if let ExprKind::Path(None, path) = &this.kind
&& path.is_potential_trivial_const_arg()
Expand All @@ -1206,6 +1199,17 @@ impl Expr {
}
}

pub fn maybe_unwrap_block(&self) -> &Expr {
if let ExprKind::Block(block, None) = &self.kind
&& let [stmt] = block.stmts.as_slice()
&& let StmtKind::Expr(expr) = &stmt.kind
{
expr
} else {
self
}
}

pub fn to_bound(&self) -> Option<GenericBound> {
match &self.kind {
ExprKind::Path(None, path) => Some(GenericBound::Trait(
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_ast_lowering/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,9 +220,7 @@ impl<'a, 'hir> LoweringContext<'a, '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 !self.tcx.features().const_arg_path
|| !expr.is_potential_trivial_const_arg()
{
if !expr.is_potential_trivial_const_arg() {
self.create_def(
parent_def_id,
node_id,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let node_id = self.next_node_id();

// HACK(min_generic_const_args): see lower_anon_const
if !self.tcx.features().const_arg_path || !arg.is_potential_trivial_const_arg() {
if !arg.is_potential_trivial_const_arg() {
// Add a definition for the in-band const def.
self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span);
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2335,7 +2335,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
span: Span,
) -> &'hir hir::ConstArg<'hir> {
let ct_kind = match res {
Res::Def(DefKind::ConstParam, _) if self.tcx.features().const_arg_path => {
Res::Def(DefKind::ConstParam, _) => {
let qpath = self.lower_qpath(
ty_id,
&None,
Expand Down Expand Up @@ -2410,8 +2410,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.resolver.get_partial_res(expr.id).and_then(|partial_res| partial_res.full_res());
debug!("res={:?}", maybe_res);
// FIXME(min_generic_const_args): for now we only lower params to ConstArgKind::Path
if self.tcx.features().const_arg_path
&& let Some(res) = maybe_res
if let Some(res) = maybe_res
&& let Res::Def(DefKind::ConstParam, _) = res
&& let ExprKind::Path(qself, path) = &expr.kind
{
Expand Down Expand Up @@ -2442,7 +2441,7 @@ 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 self.tcx.features().const_arg_path && c.value.is_potential_trivial_const_arg() {
if c.value.is_potential_trivial_const_arg() {
// 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)
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,6 @@ declare_features! (
(unstable, anonymous_lifetime_in_impl_trait, "1.63.0", None),
/// Allows identifying the `compiler_builtins` crate.
(internal, compiler_builtins, "1.13.0", None),
/// Gating for a new desugaring of const arguments of usages of const parameters
(internal, const_arg_path, "1.81.0", None),
/// Allows writing custom MIR
(internal, custom_mir, "1.65.0", None),
/// Outputs useful `assert!` messages
Expand Down
18 changes: 5 additions & 13 deletions compiler/rustc_middle/src/ty/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,20 +295,12 @@ impl<'tcx> Const<'tcx> {
_ => expr,
};

if let hir::ExprKind::Path(
qpath @ hir::QPath::Resolved(
_,
&hir::Path { res: Res::Def(DefKind::ConstParam, _), .. },
),
) = expr.kind
if let hir::ExprKind::Path(hir::QPath::Resolved(
_,
&hir::Path { res: Res::Def(DefKind::ConstParam, _), .. },
)) = expr.kind
{
if tcx.features().const_arg_path {
span_bug!(
expr.span,
"try_from_lit: received const param which shouldn't be possible"
);
}
return Some(Const::from_param(tcx, qpath, expr.hir_id));
span_bug!(expr.span, "try_from_lit: received const param which shouldn't be possible");
};

let lit_input = match expr.kind {
Expand Down
120 changes: 80 additions & 40 deletions compiler/rustc_resolve/src/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,37 @@ use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::Span;
use tracing::debug;

use crate::{ImplTraitContext, Resolver};
use crate::{ImplTraitContext, InvocationParent, PendingAnonConstInfo, Resolver};

pub(crate) fn collect_definitions(
resolver: &mut Resolver<'_, '_>,
fragment: &AstFragment,
expansion: LocalExpnId,
) {
let (parent_def, impl_trait_context, in_attr) = resolver.invocation_parents[&expansion];
let mut visitor = DefCollector { resolver, parent_def, expansion, impl_trait_context, in_attr };
let InvocationParent { parent_def, pending_anon_const_info, 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,
};
fragment.visit_with(&mut visitor);
}

/// Creates `DefId`s for nodes in the AST.
struct DefCollector<'a, 'b, 'tcx> {
resolver: &'a mut Resolver<'b, '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<PendingAnonConstInfo>,
impl_trait_context: ImplTraitContext,
in_attr: bool,
expansion: LocalExpnId,
Expand Down Expand Up @@ -111,10 +126,16 @@ impl<'a, 'b, 'tcx> DefCollector<'a, 'b, 'tcx> {

fn visit_macro_invoc(&mut self, id: NodeId) {
let id = id.placeholder_to_expn_id();
let old_parent = self
.resolver
.invocation_parents
.insert(id, (self.parent_def, self.impl_trait_context, self.in_attr));
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");
}
}
Expand Down Expand Up @@ -326,46 +347,65 @@ impl<'a, 'b, 'tcx> visit::Visitor<'a> for DefCollector<'a, 'b, 'tcx> {
}

fn visit_anon_const(&mut self, constant: &'a AnonConst) {
if self.resolver.tcx.features().const_arg_path
&& constant.value.is_potential_trivial_const_arg()
{
// 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.
visit::walk_anon_const(self, constant)
} else {
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));
// 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 matches!(constant.value.maybe_unwrap_block().kind, ExprKind::MacCall(..)) {
// See self.pending_anon_const_info for explanation
self.pending_anon_const_info =
Some(PendingAnonConstInfo { id: constant.id, span: constant.value.span });
return visit::walk_anon_const(self, constant);
} else if constant.value.is_potential_trivial_const_arg() {
return visit::walk_anon_const(self, constant);
}

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) {
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);
}
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;
if matches!(expr.kind, ExprKind::MacCall(..)) {
return self.visit_macro_invoc(expr.id);
}

let grandparent_def = if let Some(pending_anon) = self.pending_anon_const_info.take() {
// See self.pending_anon_const_info for explanation
if !expr.is_potential_trivial_const_arg() {
self.create_def(pending_anon.id, kw::Empty, DefKind::AnonConst, pending_anon.span)
} else {
self.parent_def
}
_ => self.parent_def,
} else {
self.parent_def
};

self.with_parent(parent_def, |this| visit::walk_expr(this, expr));
self.with_parent(grandparent_def, |this| {
let parent_def = match expr.kind {
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;
}
_ => this.parent_def,
};

this.with_parent(parent_def, |this| visit::walk_expr(this, expr))
})
}

fn visit_ty(&mut self, ty: &'a Ty) {
Expand Down
28 changes: 25 additions & 3 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,29 @@ impl<'a> ParentScope<'a> {
}
}

#[derive(Copy, Debug, Clone)]
struct InvocationParent {
parent_def: LocalDefId,
pending_anon_const_info: Option<PendingAnonConstInfo>,
impl_trait_context: ImplTraitContext,
in_attr: bool,
}

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 {
id: NodeId,
span: Span,
}

#[derive(Copy, Debug, Clone)]
enum ImplTraitContext {
Existential,
Expand Down Expand Up @@ -1144,7 +1167,7 @@ pub struct Resolver<'a, 'tcx> {
/// When collecting definitions from an AST fragment produced by a macro invocation `ExpnId`
/// we know what parent node that fragment should be attached to thanks to this table,
/// and how the `impl Trait` fragments were introduced.
invocation_parents: FxHashMap<LocalExpnId, (LocalDefId, ImplTraitContext, bool /*in_attr*/)>,
invocation_parents: FxHashMap<LocalExpnId, InvocationParent>,

/// Some way to know that we are in a *trait* impl in `visit_assoc_item`.
/// FIXME: Replace with a more general AST map (together with some other fields).
Expand Down Expand Up @@ -1381,8 +1404,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
node_id_to_def_id.insert(CRATE_NODE_ID, crate_feed);

let mut invocation_parents = FxHashMap::default();
invocation_parents
.insert(LocalExpnId::ROOT, (CRATE_DEF_ID, ImplTraitContext::Existential, false));
invocation_parents.insert(LocalExpnId::ROOT, InvocationParent::ROOT);

let mut extern_prelude: FxHashMap<Ident, ExternPreludeEntry<'_>> = tcx
.sess
Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ use crate::errors::{
use crate::imports::Import;
use crate::Namespace::*;
use crate::{
BindingKey, BuiltinMacroState, DeriveData, Determinacy, Finalize, MacroData, ModuleKind,
ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope, PathResult, ResolutionError,
Resolver, ScopeSet, Segment, ToNameBinding, Used,
BindingKey, BuiltinMacroState, DeriveData, Determinacy, Finalize, InvocationParent, MacroData,
ModuleKind, ModuleOrUniformRoot, NameBinding, NameBindingKind, ParentScope, PathResult,
ResolutionError, Resolver, ScopeSet, Segment, ToNameBinding, Used,
};

type Res = def::Res<NodeId>;
Expand Down Expand Up @@ -183,7 +183,7 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
}

fn invocation_parent(&self, id: LocalExpnId) -> LocalDefId {
self.invocation_parents[&id].0
self.invocation_parents[&id].parent_def
}

fn resolve_dollar_crates(&mut self) {
Expand Down Expand Up @@ -303,12 +303,12 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
.invocation_parents
.get(&invoc_id)
.or_else(|| self.invocation_parents.get(&eager_expansion_root))
.filter(|&&(mod_def_id, _, in_attr)| {
.filter(|&&InvocationParent { parent_def: mod_def_id, in_attr, .. }| {
in_attr
&& invoc.fragment_kind == AstFragmentKind::Expr
&& self.tcx.def_kind(mod_def_id) == DefKind::Mod
})
.map(|&(mod_def_id, ..)| mod_def_id);
.map(|&InvocationParent { parent_def: mod_def_id, .. }| mod_def_id);
let (ext, res) = self.smart_resolve_macro_path(
path,
kind,
Expand Down Expand Up @@ -951,7 +951,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let node_id = self
.invocation_parents
.get(&parent_scope.expansion)
.map_or(ast::CRATE_NODE_ID, |id| self.def_id_to_node_id[id.0]);
.map_or(ast::CRATE_NODE_ID, |parent| {
self.def_id_to_node_id[parent.parent_def]
});
self.lint_buffer.buffer_lint(
LEGACY_DERIVE_HELPERS,
node_id,
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,6 @@ symbols! {
conservative_impl_trait,
console,
const_allocate,
const_arg_path,
const_async_blocks,
const_closures,
const_compare_raw_pointers,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui-fulldeps/stable-mir/check_instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn test_stable_mir() -> ControlFlow<()> {
// Get all items and split generic vs monomorphic items.
let (generic, mono): (Vec<_>, Vec<_>) =
items.into_iter().partition(|item| item.requires_monomorphization());
assert_eq!(mono.len(), 4, "Expected 3 mono functions");
assert_eq!(mono.len(), 3, "Expected 3 mono functions");
assert_eq!(generic.len(), 2, "Expected 2 generic functions");

// For all monomorphic items, get the correspondent instances.
Expand Down
Loading
Loading