Skip to content

Stop exporting TypeckRootCtxt and FnCtxt.#123625

Merged
bors merged 1 commit into
rust-lang:masterfrom
oli-obk:private_fnctxt
Apr 8, 2024
Merged

Stop exporting TypeckRootCtxt and FnCtxt.#123625
bors merged 1 commit into
rust-lang:masterfrom
oli-obk:private_fnctxt

Conversation

@oli-obk

@oli-obk oli-obk commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

While they have many convenient APIs, it is better to expose dedicated functions for them

noticed in #122213

@rustbot

rustbot commented Apr 8, 2024

Copy link
Copy Markdown
Collaborator

r? @davidtwco

rustbot has assigned @davidtwco.
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 Apr 8, 2024
@rustbot

rustbot commented Apr 8, 2024

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Comment on lines -63 to -64
/// whether the cast is made in a const context or not.
pub constness: hir::Constness,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also helps with the compiler telling us about unused things

@fee1-dead fee1-dead left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I hope you don't mind the comment nits. r=me afterwards :)

Comment thread compiler/rustc_hir_typeck/src/cast.rs Outdated
Comment on lines +197 to +199
/// If a cast from `from_ty` to `to_ty` is valid, returns an Ok containing the kind of
/// the cast. In certain cases, including some invalid casts from array references
/// to pointers, this may cause additional errors to be emitted and/or ICE error

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/// If a cast from `from_ty` to `to_ty` is valid, returns an Ok containing the kind of
/// the cast. In certain cases, including some invalid casts from array references
/// to pointers, this may cause additional errors to be emitted and/or ICE error
/// If a cast from `from_ty` to `to_ty` is valid, returns a `Some` containing the kind
/// of the cast. In certain cases, including some invalid casts from array references
/// to pointers, this may cause additional errors to be emitted and/or ICE error

This returns an Option, not a Result.

Comment thread compiler/rustc_hir_typeck/src/cast.rs Outdated

if let Ok(check) = CastCheck::new(
&fn_ctxt, e, from_ty, to_ty,
// We won't show any error to the user, so we don't care what the span is here.

@fee1-dead fee1-dead Apr 8, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// We won't show any error to the user, so we don't care what the span is here.
// We won't show any errors to the user, so the span is irrelevant here.

Would probably read better (I know you didn't write these but seems nice to do a drive-by)

Comment thread compiler/rustc_hir_typeck/src/cast.rs Outdated
/// If a cast from `from_ty` to `to_ty` is valid, returns an Ok containing the kind of
/// the cast. In certain cases, including some invalid casts from array references
/// to pointers, this may cause additional errors to be emitted and/or ICE error
/// messages. This function will panic if that occurs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this really true? I don't see any panicking done in the body of this function, but I assume CastCheck wouldn't do that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh heh. I didn't read the comment at all and just copied over the function

@fee1-dead fee1-dead assigned fee1-dead and unassigned davidtwco Apr 8, 2024
@fee1-dead fee1-dead 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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2024
While they have many convenient APIs, it is better to expose dedicated functions for them
@oli-obk

oli-obk commented Apr 8, 2024

Copy link
Copy Markdown
Contributor Author

@bors r=fee1-dead

@bors

bors commented Apr 8, 2024

Copy link
Copy Markdown
Collaborator

📌 Commit a9edbfd has been approved by fee1-dead

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 Apr 8, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#115984 (extending filesystem support for Hermit)
 - rust-lang#120144 (privacy: Stabilize lint `unnameable_types`)
 - rust-lang#122807 (Add consistency with phrases "meantime" and "mean time")
 - rust-lang#123089 (Add invariant to VecDeque::pop_* that len < cap if pop successful)
 - rust-lang#123595 (Documentation fix)
 - rust-lang#123625 (Stop exporting `TypeckRootCtxt` and `FnCtxt`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f825271 into rust-lang:master Apr 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2024
Rollup merge of rust-lang#123625 - oli-obk:private_fnctxt, r=fee1-dead

Stop exporting `TypeckRootCtxt` and `FnCtxt`.

While they have many convenient APIs, it is better to expose dedicated functions for them

noticed in rust-lang#122213
@rustbot rustbot added this to the 1.79.0 milestone Apr 8, 2024
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 18, 2024
Stop exporting `TypeckRootCtxt` and `FnCtxt`.

While they have many convenient APIs, it is better to expose dedicated functions for them

noticed in rust-lang#122213
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.

5 participants