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.

Sign up for GitHub

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

Jump to bottom

Simplify some places that deal with generic parameter defaults #132912

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Simplify some places that deal with generic parameter defaults #132912

merged 1 commit into from
Nov 12, 2024
+39 −115

Conversation

fmease
Copy link
Member

@fmease fmease commented Nov 11, 2024

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Nov 11, 2024

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
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 Nov 11, 2024
@fmease fmease added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Nov 11, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fmease fmease marked this pull request as draft November 11, 2024 17:59
@fmease fmease force-pushed the simplify-gen-param-default-users branch from 1f80553 to 29551af Compare November 11, 2024 18:03
@fmease fmease marked this pull request as ready for review November 11, 2024 18:03
@rust-log-analyzer

This comment has been minimized.

Sign in to view
compiler-errors
compiler-errors requested changes Nov 11, 2024
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors 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 after fixing typo and green ci

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated Show resolved Hide resolved
compiler-errors
compiler-errors reviewed Nov 11, 2024
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Oh, and you need to fix the default arg logic below. I can re-review that after you've fixed it.

@fmease fmease force-pushed the simplify-gen-param-default-users branch from 29551af to d33e885 Compare November 11, 2024 19:03
@fmease
Copy link
Member Author

fmease commented Nov 11, 2024

Ah, lol. I guess there's some merit in checking if it actually compiles and in reading the rest of a function body ^^.

@fmease fmease force-pushed the simplify-gen-param-default-users branch from d33e885 to c247dec Compare November 11, 2024 20:26
@fmease

This comment was marked as outdated.

Sign in to view
@bors

This comment was marked as outdated.

Sign in to view
@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 Nov 11, 2024
@fmease

This comment was marked as outdated.

Sign in to view
@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 Nov 11, 2024
@fmease fmease force-pushed the simplify-gen-param-default-users branch from c247dec to d0ddba3 Compare November 11, 2024 20:30
@fmease
Copy link
Member Author

fmease commented Nov 11, 2024

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Nov 11, 2024

📌 Commit d0ddba3 has been approved by compiler-errors

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 Nov 11, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Nov 11, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 11, 2024
@bors
Auto merge of rust-lang#132919 - matthiaskrgr:rollup-ogghyvp, r=matth…
715c5a0 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#120077 (Add Set entry API )
 - rust-lang#132144 (Arbitrary self types v2: (unused) Receiver trait)
 - rust-lang#132297 (Document some `check_expr` methods, and other misc `hir_typeck` tweaks)
 - rust-lang#132820 (Add a default implementation for CodegenBackend::link)
 - rust-lang#132881 (triagebot: Autolabel rustdoc book)
 - rust-lang#132912 (Simplify some places that deal with generic parameter defaults)
 - rust-lang#132916 (Unvacation fmease)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 12, 2024
@bors
Auto merge of rust-lang#132919 - matthiaskrgr:rollup-ogghyvp, r=matth…
67f2127 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#120077 (Add Set entry API )
 - rust-lang#132144 (Arbitrary self types v2: (unused) Receiver trait)
 - rust-lang#132297 (Document some `check_expr` methods, and other misc `hir_typeck` tweaks)
 - rust-lang#132820 (Add a default implementation for CodegenBackend::link)
 - rust-lang#132881 (triagebot: Autolabel rustdoc book)
 - rust-lang#132912 (Simplify some places that deal with generic parameter defaults)
 - rust-lang#132916 (Unvacation fmease)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b41baf8 into rust-lang:master Nov 12, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 12, 2024
@rust-timer
Unrolled build for rust-lang#132912
7081e9b 
Rollup merge of rust-lang#132912 - fmease:simplify-gen-param-default-users, r=compiler-errors

Simplify some places that deal with generic parameter defaults
@fmease fmease deleted the simplify-gen-param-default-users branch November 12, 2024 06:41
petrochenkov
petrochenkov reviewed Nov 12, 2024
View reviewed changes
compiler/rustc_privacy/src/lib.rs
if let GenericParamDefKind::Const { .. } = param.kind {
self.visit(self.ev.tcx.type_of(param.def_id).instantiate_identity());
}
if let Some(default) = param.default_value(self.ev.tcx) {
Copy link
Contributor

@petrochenkov petrochenkov Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The default_value naming is pretty misleading, it looks like it only returns something for constant parameters (because only they has values).
I thought the GenericParamDefKind::Type was lost here before I looked at the definition.

Copy link
Member Author

Choose a reason for hiding this comment

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

How do you feel about renaming it to default?

Copy link
Member Author

@fmease fmease Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Alternatively, default_term (cc, {ast, hir}::Term) or default_arg?

Copy link
Member

Choose a reason for hiding this comment

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

I think default_term would be fine and probably actually an improvement, but also I don't know if I share the same concern about the default_value name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BoxyUwU BoxyUwU BoxyUwU left review comments

@petrochenkov petrochenkov petrochenkov left review comments

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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
Milestone
1.84.0
Development

Successfully merging this pull request may close these issues.
7 participants
@fmease @rustbot @rust-log-analyzer @bors @compiler-errors @petrochenkov @BoxyUwU