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

add some comments to hir::ModuleItems #122771

Merged
merged 2 commits into from
Mar 21, 2024
Merged

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Mar 20, 2024

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

@rustbot
Copy link
Collaborator

rustbot commented Mar 20, 2024

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

@rustbot 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 Mar 20, 2024
/// 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> + '_ {
Copy link
Contributor

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?

Copy link
Member Author

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?

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Member

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 :)

Copy link
Member Author

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.

Copy link
Member Author

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?

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@oli-obk oli-obk left a comment

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

@RalfJung
Copy link
Member Author

@bors r=oli-obk rollup

@bors
Copy link
Contributor

bors commented Mar 21, 2024

📌 Commit 1933969 has been approved by oli-obk

It is now in the queue for this repository.

@bors 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 Mar 21, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 21, 2024
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 Mar 21, 2024
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 Mar 21, 2024
…kingjubilee

Rollup of 9 pull requests

Successful merges:

 - rust-lang#122222 (deref patterns: bare-bones feature gate and typechecking)
 - rust-lang#122456 (CFI: Skip non-passed arguments)
 - rust-lang#122696 (Add bare metal riscv32 target.)
 - rust-lang#122771 (add some comments to hir::ModuleItems)
 - 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)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 21, 2024
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
Copy link
Contributor

bors commented Mar 21, 2024

☔ The latest upstream changes (presumably #122568) made this pull request unmergeable. Please resolve the merge conflicts.

@bors 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 Mar 21, 2024
@RalfJung
Copy link
Member Author

@bors r=oli-obk rollup

@bors
Copy link
Contributor

bors commented Mar 21, 2024

📌 Commit 0dd8a83 has been approved by oli-obk

It is now in the queue for this repository.

@bors 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 Mar 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2024
…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 Mar 21, 2024
…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
@bors bors merged commit 7938ce6 into rust-lang:master Mar 21, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2024
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.
@RalfJung RalfJung deleted the module-items branch March 22, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants