add some comments to hir::ModuleItems by RalfJung · Pull Request #122771 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation15 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

I've definitely been bitten by this in the past, where I assumed items() would give me all the items.

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

rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

/// Returns all non-associated locally defined items in all modules.
///
/// Note that this does not include associated items of `impl` blocks! It also does not
/// include foreign items. If you want to e.g. get all functions, use `definitions()` below.
pub fn items(&self) -> impl Iterator<Item = ItemId> + '_ {

Choose a reason for hiding this comment

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

maybe rename it to free_items?

Choose a reason for hiding this comment

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

Does that term have any precedent in the compiler?

Choose a reason for hiding this comment

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

there are 56 results for free function and 6 results for free const, among them results in diagnostic messages.

Choose a reason for hiding this comment

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

and resolve_instance refers to free item in debug logging

Choose a reason for hiding this comment

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

Yeah, free would be a good descriptor here. Let's help establish precedence here :)

Choose a reason for hiding this comment

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

Okay. :)

"free" captures "non-assoc" but doesn't capture "also excludes foreign items". But I think that's less important to capture anyway.

Choose a reason for hiding this comment

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

Also it seems to include the impl blocks themselves, or did I misunderstand that?

This comment has been minimized.

Choose a reason for hiding this comment

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

r=me with CI happy

📌 Commit 1933969 has been approved by oli-obk

It is now in the queue for this repository.

bors 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

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

add some comments to hir::ModuleItems

I've definitely been bitten by this in the past, where I assumed items() would give me all the items.

workingjubilee added a commit to workingjubilee/rustc that referenced this pull request

add some comments to hir::ModuleItems

I've definitely been bitten by this in the past, where I assumed items() would give me all the items.

bors added a commit to rust-lang-ci/rust that referenced this pull request

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

add some comments to hir::ModuleItems

I've definitely been bitten by this in the past, where I assumed items() would give me all the items.

This was referenced

Mar 21, 2024

bors 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

📌 Commit 0dd8a83 has been approved by oli-obk

It is now in the queue for this repository.

bors 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-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

bors added a commit to rust-lang-ci/rust that referenced this pull request

…iaskrgr

Rollup of 8 pull requests

Successful merges:

rust-lang#122222 (deref patterns: bare-bones feature gate and typechecking)
rust-lang#122358 (Don't ICE when encountering bound regions in generator interior type)
rust-lang#122696 (Add bare metal riscv32 target.)
rust-lang#122773 (make "expected paren or brace" error translatable)
rust-lang#122795 (Inherit RUSTC_BOOTSTRAP when testing wasm)
rust-lang#122799 (Replace closures with _ when suggesting fully qualified path for method call)
rust-lang#122801 (Fix misc printing issues in emit=stable_mir)
rust-lang#122806 (Make type_ascribe! not a built-in)

Failed merges:

rust-lang#122771 (add some comments to hir::ModuleItems)

r? @ghost @rustbot modify labels: rollup

bors added a commit to rust-lang-ci/rust that referenced this pull request

…iaskrgr

Rollup of 8 pull requests

Successful merges:

rust-lang#122402 (Make #[diagnostic::on_unimplemented] format string parsing more robust)
rust-lang#122644 (pattern analysis: add a custom test harness)
rust-lang#122733 (Strip placeholders from hidden types before remapping generic parameter)
rust-lang#122752 (Interpolated cleanups)
rust-lang#122771 (add some comments to hir::ModuleItems)
rust-lang#122793 (Implement macro-based deref!() syntax for deref patterns)
rust-lang#122810 (Remove target_override)
rust-lang#122827 (Remove unnecessary braces from bug/span_bug)

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Rollup merge of rust-lang#122771 - RalfJung:module-items, r=oli-obk

add some comments to hir::ModuleItems

I've definitely been bitten by this in the past, where I assumed items() would give me all the items.

Labels

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Relevant to the compiler team, which will review and decide on the PR/issue.