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

rustc: Make def_kind mandatory for all DefIds #118250

Merged
merged 1 commit into from
Nov 26, 2023
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
15 changes: 6 additions & 9 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ impl MetadataBlob {
) -> io::Result<()> {
let root = blob.get_root();

let def_kind = root.tables.opt_def_kind.get(blob, item).unwrap();
let def_kind = root.tables.def_kind.get(blob, item).unwrap();
let def_key = root.tables.def_keys.get(blob, item).unwrap().decode(blob);
let def_name = if item == CRATE_DEF_INDEX {
rustc_span::symbol::kw::Crate
Expand Down Expand Up @@ -1001,14 +1001,11 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
}

fn def_kind(self, item_id: DefIndex) -> DefKind {
self.root.tables.opt_def_kind.get(self, item_id).unwrap_or_else(|| {
bug!(
"CrateMetadata::def_kind({:?}): id not found, in crate {:?} with number {}",
item_id,
self.root.name(),
self.cnum,
)
})
self.root
.tables
.def_kind
.get(self, item_id)
.unwrap_or_else(|| self.missing("def_kind", item_id))
}

fn get_span(self, index: DefIndex, sess: &Session) -> Span {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ provide! { tcx, def_id, other, cdata,
lookup_deprecation_entry => { table }
params_in_repr => { table }
unused_generic_params => { cdata.root.tables.unused_generic_params.get(cdata, def_id.index) }
opt_def_kind => { table_direct }
def_kind => { cdata.def_kind(def_id.index) }
impl_parent => { table }
impl_polarity => { table_direct }
defaultness => { table_direct }
Expand Down
19 changes: 12 additions & 7 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1354,9 +1354,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {

for local_id in tcx.iter_local_def_id() {
let def_id = local_id.to_def_id();
let def_kind = tcx.opt_def_kind(local_id);
let Some(def_kind) = def_kind else { continue };
self.tables.opt_def_kind.set_some(def_id.index, def_kind);
let def_kind = tcx.def_kind(local_id);
self.tables.def_kind.set_some(def_id.index, def_kind);
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 Expand Up @@ -1393,7 +1392,13 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
if should_encode_fn_sig(def_kind) {
record!(self.tables.fn_sig[def_id] <- tcx.fn_sig(def_id));
}
if should_encode_generics(def_kind) {
// FIXME: Some anonymous constants produced by `#[rustc_legacy_const_generics]`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we give them a dummy hir node to avoid those ices?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, I just didn't want to get into that in this PR, #118188 is not blocked on it.

I tried to look what exactly happens with rustc_legacy_const_generics, but it's still not clear to me.
There seems to be some mismatch between what def collector and AST lowering do.

// do not have corresponding HIR nodes, so some queries usually making sense for
// anonymous constants will not work on them and panic. It's not clear whether it
// can cause any observable issues or not.
let anon_const_without_hir = def_kind == DefKind::AnonConst
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
&& tcx.hir().find(tcx.local_def_id_to_hir_id(local_id)).is_none();
if should_encode_generics(def_kind) && !anon_const_without_hir {
let g = tcx.generics_of(def_id);
record!(self.tables.generics_of[def_id] <- g);
record!(self.tables.explicit_predicates_of[def_id] <- self.tcx.explicit_predicates_of(def_id));
Expand All @@ -1407,7 +1412,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}
}
if should_encode_type(tcx, local_id, def_kind) {
if should_encode_type(tcx, local_id, def_kind) && !anon_const_without_hir {
record!(self.tables.type_of[def_id] <- self.tcx.type_of(def_id));
}
if should_encode_constness(def_kind) {
Expand Down Expand Up @@ -1785,7 +1790,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
self.tables.proc_macro_quoted_spans.set_some(i, span);
}

self.tables.opt_def_kind.set_some(LOCAL_CRATE.as_def_id().index, DefKind::Mod);
self.tables.def_kind.set_some(LOCAL_CRATE.as_def_id().index, DefKind::Mod);
record!(self.tables.def_span[LOCAL_CRATE.as_def_id()] <- tcx.def_span(LOCAL_CRATE.as_def_id()));
self.encode_attrs(LOCAL_CRATE.as_def_id().expect_local());
let vis = tcx.local_visibility(CRATE_DEF_ID).map_id(|def_id| def_id.local_def_index);
Expand Down Expand Up @@ -1833,7 +1838,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
def_key.disambiguated_data.data = DefPathData::MacroNs(name);

let def_id = id.to_def_id();
self.tables.opt_def_kind.set_some(def_id.index, DefKind::Macro(macro_kind));
self.tables.def_kind.set_some(def_id.index, DefKind::Macro(macro_kind));
self.tables.proc_macro.set_some(def_id.index, macro_kind);
self.encode_attrs(id);
record!(self.tables.def_keys[def_id] <- def_key);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ define_tables! {
// so we can take their names, visibilities etc from other encoded tables.
module_children_non_reexports: Table<DefIndex, LazyArray<DefIndex>>,
associated_item_or_field_def_ids: Table<DefIndex, LazyArray<DefIndex>>,
opt_def_kind: Table<DefIndex, DefKind>,
def_kind: Table<DefIndex, DefKind>,
visibility: Table<DefIndex, LazyValue<ty::Visibility<DefIndex>>>,
def_span: Table<DefIndex, LazyValue<Span>>,
def_ident_span: Table<DefIndex, LazyValue<Span>>,
Expand Down
18 changes: 7 additions & 11 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,21 +174,18 @@ impl<'hir> Map<'hir> {
}

/// Do not call this function directly. The query should be called.
pub(super) fn opt_def_kind(self, local_def_id: LocalDefId) -> Option<DefKind> {
pub(super) fn def_kind(self, local_def_id: LocalDefId) -> DefKind {
let hir_id = self.local_def_id_to_hir_id(local_def_id);
let node = match self.find(hir_id) {
Some(node) => node,
None => match self.def_key(local_def_id).disambiguated_data.data {
// FIXME: Some anonymous constants do not have corresponding HIR nodes,
// so many local queries will panic on their def ids. `None` is currently
// returned here instead of `DefKind::{Anon,Inline}Const` to avoid such panics.
// Ideally all def ids should have `DefKind`s, we need to create the missing
// HIR nodes or feed relevant query results to achieve that.
DefPathData::AnonConst => return None,
// FIXME: Some anonymous constants produced by `#[rustc_legacy_const_generics]`
// do not have corresponding HIR nodes, but they are still anonymous constants.
DefPathData::AnonConst => return DefKind::AnonConst,
_ => bug!("no HIR node for def id {local_def_id:?}"),
},
};
let def_kind = match node {
match node {
Node::Item(item) => match item.kind {
ItemKind::Static(_, mt, _) => DefKind::Static(mt),
ItemKind::Const(..) => DefKind::Const,
Expand Down Expand Up @@ -266,8 +263,7 @@ impl<'hir> Map<'hir> {
self.span(hir_id),
"unexpected node with def id {local_def_id:?}: {node:?}"
),
};
Some(def_kind)
}
}

/// Finds the id of the parent node to this one.
Expand Down Expand Up @@ -895,7 +891,7 @@ impl<'hir> Map<'hir> {

#[inline]
fn opt_ident(self, id: HirId) -> Option<Ident> {
match self.get(id) {
match self.find(id)? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What codepath triggers this specifically? Just curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def_span for anonymous constants without HIR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep a FIXME here too?

Copy link
Contributor Author

@petrochenkov petrochenkov Nov 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HirIds do not always have HIR nodes or Idents, so the new behavior looks reasonable to me, we just didn't hit this case before.

Node::Pat(&Pat { kind: PatKind::Binding(_, _, ident, _), .. }) => Some(ident),
// A `Ctor` doesn't have an identifier itself, but its parent
// struct/variant does. Compare with `hir::Map::opt_span`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ pub fn provide(providers: &mut Providers) {
span_bug!(hir.span(hir_id), "fn_arg_names: unexpected item {:?}", def_id);
}
};
providers.opt_def_kind = |tcx, def_id| tcx.hir().opt_def_kind(def_id);
providers.def_kind = |tcx, def_id| tcx.hir().def_kind(def_id);
providers.all_local_trait_impls = |tcx, ()| &tcx.resolutions(()).trait_impls;
providers.expn_that_defined =
|tcx, id| tcx.resolutions(()).expn_that_defined.get(&id).copied().unwrap_or(ExpnId::root());
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1151,7 +1151,7 @@ rustc_queries! {
cache_on_disk_if { true }
}

query opt_def_kind(def_id: DefId) -> Option<DefKind> {
query def_kind(def_id: DefId) -> DefKind {
desc { |tcx| "looking up definition kind of `{}`", tcx.def_path_str(def_id) }
cache_on_disk_if { def_id.is_local() }
separate_provide_extern
Expand Down
17 changes: 0 additions & 17 deletions compiler/rustc_middle/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use measureme::StringId;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::AtomicU64;
use rustc_data_structures::sync::WorkerLocal;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::hir_id::OwnerId;
use rustc_query_system::dep_graph::DepNodeIndex;
Expand Down Expand Up @@ -668,21 +667,5 @@ mod sealed {

pub use sealed::IntoQueryParam;

impl<'tcx> TyCtxt<'tcx> {
pub fn def_kind(self, def_id: impl IntoQueryParam<DefId>) -> DefKind {
let def_id = def_id.into_query_param();
self.opt_def_kind(def_id)
.unwrap_or_else(|| bug!("def_kind: unsupported node: {:?}", def_id))
}
}

impl<'tcx> TyCtxtAt<'tcx> {
pub fn def_kind(self, def_id: impl IntoQueryParam<DefId>) -> DefKind {
let def_id = def_id.into_query_param();
self.opt_def_kind(def_id)
.unwrap_or_else(|| bug!("def_kind: unsupported node: {:?}", def_id))
}
}

#[derive(Copy, Clone, Debug, HashStable)]
pub struct CyclePlaceholder(pub ErrorGuaranteed);
2 changes: 1 addition & 1 deletion compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ impl<'tcx> EmbargoVisitor<'tcx> {
}

let macro_module_def_id = self.tcx.local_parent(local_def_id);
if self.tcx.opt_def_kind(macro_module_def_id) != Some(DefKind::Mod) {
if self.tcx.def_kind(macro_module_def_id) != DefKind::Mod {
// The macro's parent doesn't correspond to a `mod`, return early (#63164, #65252).
return;
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,11 @@ pub(crate) fn create_query_frame<
Some(key.default_span(tcx))
};
let def_id = key.key_as_def_id();
let def_kind = if kind == dep_graph::dep_kinds::opt_def_kind || with_no_queries() {
let def_kind = if kind == dep_graph::dep_kinds::def_kind || with_no_queries() {
// Try to avoid infinite recursion.
None
} else {
def_id.and_then(|def_id| def_id.as_local()).and_then(|def_id| tcx.opt_def_kind(def_id))
def_id.and_then(|def_id| def_id.as_local()).map(|def_id| tcx.def_kind(def_id))
};
let hash = || {
tcx.with_stable_hashing_context(|mut hcx| {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ty_utils/src/assoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ fn associated_type_for_impl_trait_in_trait(
let local_def_id = trait_assoc_ty.def_id();
let def_id = local_def_id.to_def_id();

trait_assoc_ty.opt_def_kind(Some(DefKind::AssocTy));
trait_assoc_ty.def_kind(DefKind::AssocTy);

// There's no HIR associated with this new synthesized `def_id`, so feed
// `opt_local_def_id_to_hir_id` with `None`.
Expand Down Expand Up @@ -362,7 +362,7 @@ fn associated_type_for_impl_trait_in_impl(
let local_def_id = impl_assoc_ty.def_id();
let def_id = local_def_id.to_def_id();

impl_assoc_ty.opt_def_kind(Some(DefKind::AssocTy));
impl_assoc_ty.def_kind(DefKind::AssocTy);

// There's no HIR associated with this new synthesized `def_id`, so feed
// `opt_local_def_id_to_hir_id` with `None`.
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/unused_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
// statements, so don't lint at all if there are any such paths.
if let Some(def_id) = path.res.opt_def_id()
&& let Some(local_def_id) = def_id.as_local()
&& let Some(DefKind::Fn) = cx.tcx.opt_def_kind(def_id)
&& cx.tcx.def_kind(def_id) == DefKind::Fn
&& cx.tcx.asyncness(def_id).is_async()
&& !is_node_func_call(cx.tcx.hir().get_parent(hir_id), path.span)
{
Expand Down
3 changes: 1 addition & 2 deletions src/tools/clippy/clippy_lints/src/useless_conversion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
use clippy_utils::{get_parent_expr, is_trait_method, is_ty_alias, path_to_local};
use rustc_errors::Applicability;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
use rustc_infer::infer::TyCtxtInferExt;
Expand Down Expand Up @@ -208,7 +207,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
&& let Some(did) = cx.qpath_res(qpath, recv.hir_id).opt_def_id()
// make sure that the path indeed points to a fn-like item, so that
// `fn_sig` does not ICE. (see #11065)
&& cx.tcx.opt_def_kind(did).is_some_and(DefKind::is_fn_like) =>
&& cx.tcx.def_kind(did).is_fn_like() =>
{
Some((
did,
Expand Down
Loading