Replace `unwrap` with `expect` in `get_module_children` by bushrat011899 · Pull Request #158002 · rust-lang/rust

bushrat011899 · 2026-06-17T03:50:37Z

Objective

I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children calling unwrap. The local value non_reexports can be None when the passed in DefIndex is for a non-module-like item. The documentation for get_module_children implies that it must only be called with module-like item indices, however the use of unwrap makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on TyCtxt::module_children.

Solution

Switched from an Option::unwrap to a Option::expect
Added panic documentation to get_module_children
Replicated documentation for TyCtxt::module_children

Notes

No AI tooling of any kind was used during the creation of this PR.
Please see Zulip for some discussion.
Please see this failed Clippy CI run for an example of the ICE caused.

rustbot · 2026-06-17T03:50:43Z

r? @oli-obk

rustbot has assigned @oli-obk.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 73 candidates
Random selection from 20 candidates

oli-obk · 2026-06-17T05:26:46Z

@bors r+ rollup

rust-bors · 2026-06-17T05:26:48Z

📌 Commit b6fc0c3 has been approved by oli-obk

It is now in the queue for this repository.

rust-bors · 2026-06-17T07:02:10Z

⌛ Testing commit b6fc0c3 with merge e68a2da...

Workflow: https://github.com/rust-lang/rust/actions/runs/27671724601

…li-obk Fix(ICE): Remove `unwrap` from `get_module_children` # Objective Fix an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however it seems to me that it would be appropriate to just return nothing when called on a non-module-like item (if it isn't module-like, it has zero children). ## Solution - Switched from an `Option::unwrap` to an `if let Some(...)` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.

petrochenkov · 2026-06-17T09:29:39Z

@bors r-

rust-bors · 2026-06-17T09:29:45Z

This pull request was unapproved.

Auto build was cancelled due to unapproval. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/27671724601

View changes since this unapproval

petrochenkov · 2026-06-17T09:35:51Z

The local value non_reexports can be None when the passed in DefIndex is for a non-module-like item.

So the fix is to only call it on module-like items in clippy.

The documentation for get_module_children implies that it must only be called with module-like item indices

Yes, that's a very typical behavior for rustc_metadata APIs intended to prevent mistakes.
The documentation is quite explicit about the input requirements, but it can be made even more explicit by saying that the method will panic if the input is not module-like.

however it seems to me that it would be appropriate to just return nothing when called on a non-module-like item (if it isn't module-like, it has zero children).

I disagree, calling it on something that is not a module means misunderstanding and a potential mistake.

bushrat011899 · 2026-06-17T10:18:19Z

The local value non_reexports can be None when the passed in DefIndex is for a non-module-like item.

So the fix is to only call it on module-like items in clippy.

That's what I've done in my Clippy PR, but it took me a while to work out what was going wrong. Mostly because I was using this method indirectly via TyCtxt::module_children, which has no documentation, let alone a warning around panics.

The documentation for get_module_children implies that it must only be called with module-like item indices

Yes, that's a very typical behavior for rustc_metadata APIs intended to prevent mistakes. The documentation is quite explicit about the input requirements, but it can be made even more explicit by saying that the method will panic if the input is not module-like.

If panicking is absolutely the preferred behaviour, I'm happy to amend this PR to add a # Panic documentation section and replace the unwrap with an expect that reiterates the requirements.

however it seems to me that it would be appropriate to just return nothing when called on a non-module-like item (if it isn't module-like, it has zero children).

I disagree, calling it on something that is not a module means misunderstanding and a potential mistake.

That's perfectly reasonable, I disagree but I can understand why that'd be the preferred strategy here.

Also replicated documentation on `TyCtxt::module_children`

bushrat011899 · 2026-06-17T10:49:03Z

@petrochenkov I'd appreciate your feedback on this one if you wouldn't mind! I believe this now addresses yours and my concerns.

petrochenkov · 2026-06-17T11:01:34Z

The comments LGTM.

bushrat011899 · 2026-06-17T22:52:38Z

@rustbot ready

oli-obk

@bors r=petrochenkov,oli-obk

View changes since this review

rust-bors · 2026-06-18T11:13:17Z

📌 Commit d8953cd has been approved by petrochenkov,oli-obk

It is now in the queue for this repository.

rust-bors · 2026-06-18T12:11:38Z

⌛ Testing commit d8953cd with merge d001527...

Workflow: https://github.com/rust-lang/rust/actions/runs/27758529074

…etrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.

JonathanBrouwer · 2026-06-18T13:04:31Z

@bors yield
Yielding to enclosing rollup

rust-bors · 2026-06-18T13:04:36Z

Auto build was cancelled. Cancelled workflows:

https://github.com/rust-lang/rust/actions/runs/27758529074

The next pull request likely to be tested is #157309.

…unwrap, r=petrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.

…uwer Rollup of 5 pull requests Successful merges: - #157935 (Make `proc_macro::ConversionErrorKind` non exhaustive) - #158002 (Replace `unwrap` with `expect` in `get_module_children`) - #158071 (Update actions/checkout action to v6) - #158072 (Bump thin-vec to 0.2.18 to address RUSTSEC-2026-0103) - #158077 (rustdoc-json-types: Replace bincode dev-dependency with postcard)

…unwrap, r=petrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.

…uwer Rollup of 10 pull requests Successful merges: - #158026 (`RegionValues`: disable unnecessary range check) - #156795 (Handle generic reborrow in expression-use adjustment walking) - #157694 (Enhance documentation on wake call memory ordering) - #157935 (Make `proc_macro::ConversionErrorKind` non exhaustive) - #158002 (Replace `unwrap` with `expect` in `get_module_children`) - #158034 (Fix reborrow source expression visits) - #158072 (Bump thin-vec to 0.2.18 to address RUSTSEC-2026-0103) - #158074 (Document transient connection errors from TcpListener::accept) - #158077 (rustdoc-json-types: Replace bincode dev-dependency with postcard) - #158086 (renovate: Loosen dashboard approval and adopt recommended config)

Rollup of 12 pull requests Successful merges: - #156795 (Handle generic reborrow in expression-use adjustment walking) - #157694 (Enhance documentation on wake call memory ordering) - #157935 (Make `proc_macro::ConversionErrorKind` non exhaustive) - #158002 (Replace `unwrap` with `expect` in `get_module_children`) - #158009 (Reject `impl const Trait` since the right syntax is `const impl Trait` now) - #158034 (Fix reborrow source expression visits) - #158072 (Bump thin-vec to 0.2.18 to address RUSTSEC-2026-0103) - #158074 (Document transient connection errors from TcpListener::accept) - #158077 (rustdoc-json-types: Replace bincode dev-dependency with postcard) - #158086 (renovate: Loosen dashboard approval and adopt recommended config) - #158088 (codegen_ssa: no dbginfo for scalable vec local w/ `-O0`) - #158089 (Fix invalid "jump-to-def" doc link generation when an item has a `derive` proc-macro)

Rollup merge of #158002 - bushrat011899:get_module_children_unwrap, r=petrochenkov,oli-obk Replace `unwrap` with `expect` in `get_module_children` # Objective I ran into an ICE I ran into while developing rust-lang/rust-clippy#17252 that stems from `rustc_metadata::rmeta::decoder::CrateMetadata::get_module_children` calling `unwrap`. The local value `non_reexports` can be `None` when the passed in `DefIndex` is for a non-module-like item. The documentation for `get_module_children` *implies* that it must only be called with module-like item indices, however the use of `unwrap` makes diagnosing the ICE trickier than it has to be. Additionally, this behaviour is not documented on `TyCtxt::module_children`. ## Solution - Switched from an `Option::unwrap` to a `Option::expect` - Added panic documentation to `get_module_children` - Replicated documentation for `TyCtxt::module_children` --- ## Notes * No AI tooling of any kind was used during the creation of this PR. * Please see [Zulip](https://rust-lang.zulipchat.com/#narrow/channel/122651-general/topic/.E2.9C.94.20.60rustc.60.20ICE.20from.20.60TyCtxt.3A.3Amodule_children.60/with/603700184) for some discussion. * Please see this [failed Clippy CI run](https://github.com/rust-lang/rust-clippy/actions/runs/27596428127/job/81587609898#step:5:1) for an example of the ICE caused.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 17, 2026

rustbot assigned oli-obk Jun 17, 2026

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 17, 2026

rust-bors Bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 17, 2026

Document Panic in get_module_children

d8953cd

Also replicated documentation on `TyCtxt::module_children`

bushrat011899 force-pushed the get_module_children_unwrap branch from b6fc0c3 to d8953cd Compare June 17, 2026 10:44

bushrat011899 changed the title ~~Fix(ICE): Remove unwrap from get_module_children~~ Replace unwrap with expect in get_module_children Jun 17, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 17, 2026

oli-obk approved these changes Jun 18, 2026

View reviewed changes

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2026

JonathanBrouwer mentioned this pull request Jun 18, 2026

Rollup of 5 pull requests #158079

Closed

JonathanBrouwer mentioned this pull request Jun 18, 2026

Rollup of 10 pull requests #158095

Closed

GuillaumeGomez mentioned this pull request Jun 18, 2026

Rollup of 12 pull requests #158098

Merged

rust-bors Bot merged commit d066582 into rust-lang:main Jun 18, 2026
13 of 14 checks passed

rustbot added this to the 1.98.0 milestone Jun 18, 2026

Uh oh!

Conversation

bushrat011899 commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Notes

Uh oh!

rustbot commented Jun 17, 2026

Uh oh!

oli-obk commented Jun 17, 2026

Uh oh!

rust-bors Bot commented Jun 17, 2026

Uh oh!

rust-bors Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jun 17, 2026

Uh oh!

rust-bors Bot commented Jun 17, 2026

Uh oh!

petrochenkov commented Jun 17, 2026

Uh oh!

bushrat011899 commented Jun 17, 2026

Uh oh!

bushrat011899 commented Jun 17, 2026

Uh oh!

petrochenkov commented Jun 17, 2026

Uh oh!

bushrat011899 commented Jun 17, 2026

Uh oh!

oli-obk left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors Bot commented Jun 18, 2026

Uh oh!

rust-bors Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JonathanBrouwer commented Jun 18, 2026

Uh oh!

rust-bors Bot commented Jun 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bushrat011899 commented Jun 17, 2026 •

edited

Loading

rust-bors Bot commented Jun 17, 2026 •

edited

Loading

oli-obk left a comment •

edited by rustbot

Loading

rust-bors Bot commented Jun 18, 2026 •

edited

Loading