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

Stop exporting TypeckRootCtxt and FnCtxt. #123625

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

oli-obk
Copy link
Contributor

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

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

noticed in #122213

@rustbot
Copy link
Collaborator

rustbot commented Apr 8, 2024

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
Copy link
Collaborator

rustbot commented Apr 8, 2024

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

Copy link
Member

@fee1-dead fee1-dead left a comment

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 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
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.


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.
Copy link
Member

@fee1-dead fee1-dead Apr 8, 2024

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)

/// 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
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
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
Copy link
Contributor Author

oli-obk commented Apr 8, 2024

@bors r=fee1-dead

@bors
Copy link
Contributor

bors commented Apr 8, 2024

📌 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
11 checks passed
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