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

Create anon const defs in DefCollector unconditionally #133285

Closed
wants to merge 1 commit into from
Closed
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
27 changes: 0 additions & 27 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<GenericBound> {
match &self.kind {
ExprKind::Path(None, path) => Some(GenericBound::Trait(PolyTraitRef::new(
Expand Down
17 changes: 7 additions & 10 deletions compiler/rustc_ast_lowering/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
18 changes: 1 addition & 17 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
};
Expand All @@ -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);
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
156 changes: 26 additions & 130 deletions compiler/rustc_resolve/src/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,37 +12,23 @@ 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);
}

/// Creates `DefId`s for nodes in the AST.
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<PendingAnonConstInfo>,
impl_trait_context: ImplTraitContext,
in_attr: bool,
expansion: LocalExpnId,
Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 0 additions & 13 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,31 +174,18 @@ impl<'ra> ParentScope<'ra> {
#[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 {
// 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,
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/const-generics/early/const-arg-empty-macro-expansion-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
impl<T>
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() {}
Loading
Loading