Emit ForbiddenBound for bounds of type params when parsing higher ranked binders

[mu001999]

Fixes #149695

---

Bounds in binders are denied, hir items won't contain and index them. But nested items in the bounds will still be lowered to hir. And their parents, i.e., the block in bounds is not in hir. So that ICE happens when error handling requires visiting hir parents.

This PR collects hirless def ids and skips them when iterating parents.

This PR checks such bounds used in higher ranked binders and emit error ForbiddenBound when parsing. And make sure no such bounds appear in hir.

[rustbot]

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[mu001999]

@rustbot reroll

[jdonszelmann]

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[mu001999]

@rustbot review

[jdonszelmann]

@rusbot author

[jdonszelmann]

@rustbot blocked on #149728 (comment)

[mu001999]

Hi @jdonszelmann, in the lastest rough change, I tried to filter nested items in the bound, and not lower them to hir.

dbg! shows it works:

[compiler/rustc_ast_lowering/src/lib.rs:527:17] "skipping lowering of {:?}" = "skipping lowering of {:?}"
[compiler/rustc_ast_lowering/src/lib.rs:527:17] def_id = DefId(0:6 ~ bad_bounds[bee6]::produce::{constant#0}::A)
[compiler/rustc_ast_lowering/src/lib.rs:527:17] "skipping lowering of {:?}" = "skipping lowering of {:?}"
[compiler/rustc_ast_lowering/src/lib.rs:527:17] def_id = DefId(0:7 ~ bad_bounds[bee6]::produce::{constant#0}::A::{constructor#0})
[compiler/rustc_ast_lowering/src/lib.rs:527:17] "skipping lowering of {:?}" = "skipping lowering of {:?}"
[compiler/rustc_ast_lowering/src/lib.rs:527:17] def_id = DefId(0:8 ~ bad_bounds[bee6]::produce::{constant#0}::A::A)
[compiler/rustc_ast_lowering/src/lib.rs:527:17] "skipping lowering of {:?}" = "skipping lowering of {:?}"
[compiler/rustc_ast_lowering/src/lib.rs:527:17] def_id = DefId(0:10 ~ bad_bounds[bee6]::produce::{constant#0}::A#1)
[compiler/rustc_ast_lowering/src/lib.rs:527:17] "skipping lowering of {:?}" = "skipping lowering of {:?}"
[compiler/rustc_ast_lowering/src/lib.rs:527:17] def_id = DefId(0:11 ~ bad_bounds[bee6]::produce::{constant#0}::{impl#0})

But there still be errors, because they still have LocalDefId, so that they can be result of many other queries. For example, after the change, a new ICE appears because type_of(bad_bounds[bee6]::produce::{constant#0}::{impl#0}).

Concrete content of the new ICE is:

error: internal compiler error: compiler/rustc_middle/src/hir/mod.rs:385:32: No HirId for DefId(0:11 ~ bad_bounds[bee6]::produce::{constant#0}::{impl#0})


thread 'rustc' (6902108) panicked at compiler/rustc_middle/src/hir/mod.rs:385:32:
Box<dyn Any>
stack backtrace:
   0: std::panicking::begin_panic::<rustc_errors::ExplicitBug>
             at /rustc/1b6e21e163baa0b20f119e17e3871910978a60b6/library/std/src/panicking.rs:761:5
   1: std::panic::panic_any::<rustc_errors::ExplicitBug>
             at /rustc/1b6e21e163baa0b20f119e17e3871910978a60b6/library/std/src/panic.rs:260:5
   2: <rustc_errors::diagnostic::BugAbort as rustc_errors::diagnostic::EmissionGuarantee>::emit_producing_guarantee
             at ./compiler/rustc_errors/src/diagnostic.rs:63:9
   3: <rustc_errors::diagnostic::Diag<rustc_errors::diagnostic::BugAbort>>::emit
             at ./compiler/rustc_errors/src/diagnostic.rs:1405:9
   4: <rustc_errors::DiagCtxtHandle>::bug::<alloc::string::String>
             at ./compiler/rustc_errors/src/lib.rs:1231:30
   5: rustc_middle::util::bug::opt_span_bug_fmt::<rustc_span::span_encoding::Span>::{closure#0}
             at ./compiler/rustc_middle/src/util/bug.rs:39:48
   6: rustc_middle::ty::context::tls::with_opt::<rustc_middle::util::bug::opt_span_bug_fmt<rustc_span::span_encoding::Span>::{closure#0}, !>::{closure#0}
             at ./compiler/rustc_middle/src/ty/context/tls.rs:136:23
   7: rustc_middle::ty::context::tls::with_context_opt::<rustc_middle::ty::context::tls::with_opt<rustc_middle::util::bug::opt_span_bug_fmt<rustc_span::span_encoding::Span>::{closure#0}, !>::{closure#0}, !>
             at ./compiler/rustc_middle/src/ty/context/tls.rs:79:18
   8: rustc_middle::ty::context::tls::with_opt::<rustc_middle::util::bug::opt_span_bug_fmt<rustc_span::span_encoding::Span>::{closure#0}, !>
             at ./compiler/rustc_middle/src/ty/context/tls.rs:134:5
   9: rustc_middle::util::bug::opt_span_bug_fmt::<rustc_span::span_encoding::Span>
             at ./compiler/rustc_middle/src/util/bug.rs:33:5
  10: rustc_middle::util::bug::bug_fmt
             at ./compiler/rustc_middle/src/util/bug.rs:16:5
  11: rustc_middle::hir::provide::{closure#0}
             at ./compiler/rustc_middle/src/macros.rs:18:9
  12: <rustc_middle::hir::provide::{closure#0} as core::ops::function::FnOnce<(rustc_middle::ty::context::TyCtxt, rustc_span::def_id::LocalDefId)>>::call_once
             at /rustc/1b6e21e163baa0b20f119e17e3871910978a60b6/library/core/src/ops/function.rs:250:5
  13: rustc_query_impl::query_impl::local_def_id_to_hir_id::dynamic_query::{closure#2}::{closure#0}
             at ./compiler/rustc_query_impl/src/plumbing.rs:298:9
      [... omitted 22 frames ...]
  14: rustc_middle::query::inner::query_get_at::<rustc_data_structures::vec_cache::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 8]>, rustc_query_system::dep_graph::graph::DepNodeIndex>>
             at ./compiler/rustc_middle/src/query/inner.rs:33:17
  15: <rustc_middle::query::plumbing::TyCtxtAt>::local_def_id_to_hir_id::<rustc_span::def_id::LocalDefId>
             at ./compiler/rustc_middle/src/query/plumbing.rs:415:31
  16: <rustc_middle::ty::context::TyCtxt>::local_def_id_to_hir_id::<rustc_span::def_id::LocalDefId>
             at ./compiler/rustc_middle/src/query/plumbing.rs:406:35
  17: rustc_hir_analysis::collect::type_of::type_of::{closure#0}
             at ./compiler/rustc_hir_analysis/src/collect/type_of.rs:56:22
  18: rustc_hir_analysis::collect::type_of::type_of
             at ./compiler/rustc_hir_analysis/src/collect/type_of.rs:21:1
  19: rustc_query_impl::query_impl::type_of::dynamic_query::{closure#2}::{closure#0}
             at ./compiler/rustc_query_impl/src/plumbing.rs:302:13
      [... omitted 22 frames ...]
  20: rustc_middle::query::inner::query_get_at::<rustc_query_system::query::caches::DefIdCache<rustc_middle::query::erase::Erased<[u8; 8]>>>
             at ./compiler/rustc_middle/src/query/inner.rs:33:17
  21: <rustc_middle::query::plumbing::TyCtxtAt>::type_of::<rustc_span::def_id::DefId>
             at ./compiler/rustc_middle/src/query/plumbing.rs:415:31
  22: <rustc_middle::ty::context::TyCtxt>::type_of::<rustc_span::def_id::DefId>
             at ./compiler/rustc_middle/src/query/plumbing.rs:406:35
  23: rustc_middle::ty::trait_def::trait_impls_of_provider
             at ./compiler/rustc_middle/src/ty/trait_def.rs:227:32
  24: rustc_query_impl::query_impl::trait_impls_of::dynamic_query::{closure#2}::{closure#0}
             at ./compiler/rustc_query_impl/src/plumbing.rs:298:9
      [... omitted 22 frames ...]
  25: rustc_middle::query::inner::query_get_at::<rustc_query_system::query::caches::DefIdCache<rustc_middle::query::erase::Erased<[u8; 8]>>>
             at ./compiler/rustc_middle/src/query/inner.rs:33:17
  26: <rustc_middle::query::plumbing::TyCtxtAt>::trait_impls_of::<rustc_span::def_id::DefId>
             at ./compiler/rustc_middle/src/query/plumbing.rs:415:31
  27: <rustc_middle::ty::context::TyCtxt>::trait_impls_of::<rustc_span::def_id::DefId>
             at ./compiler/rustc_middle/src/query/plumbing.rs:406:35
  28: <rustc_middle::ty::context::TyCtxt>::all_impls
             at ./compiler/rustc_middle/src/ty/trait_def.rs:194:68

[mu001999]

I think at least it's more difficult than I expected. Do you have any suggestions?

[jdonszelmann]

Maybe that's not great for perf, mostly in visiting all those extra nodes I'm afraid

[mu001999]

Yep, I think this adds excessive overhead to the hot path.

[jdonszelmann]

@rustbot author for now. Maybe the fatal error is the way to go. @lcnr any opinions on this?

[mu001999]

@rustbot ready

[lcnr]

another alternative would be to actually lower where-bounds on binders to the HIR, or to already error on them when creating the AST before we collect the DefIds?

I personally feel like erroring pre def-collection might be better 🤔 haven't interacted with this code much so I am not sure :>

[mu001999]

to already error on them when creating the AST

Make sense, I think I could move the check in check_late_bound_lifetime_defs into parse_higher_ranked_binder, and report it as a parse error and then return empty bounds.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[rustbot]

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt