Simplify some places that deal with generic parameter defaults#132912
Merged
Conversation
Collaborator
rustbot
added
S-waiting-on-review
fmease
commented
Nov 11, 2024
fmease
commented
Nov 11, 2024
fmease
added
the
C-cleanup
This comment has been minimized.
This comment has been minimized.
fmease
force-pushed
the
simplify-gen-param-default-users
branch
from
1f80553 to
29551af
Compare
This comment has been minimized.
This comment has been minimized.
compiler-errors
suggested changes
Nov 11, 2024
compiler-errors
left a comment
compiler-errors
left a comment
Contributor
There was a problem hiding this comment.
r=me after fixing typo and green ci
compiler-errors
left a comment
compiler-errors
left a comment
Contributor
There was a problem hiding this comment.
Oh, and you need to fix the default arg logic below. I can re-review that after you've fixed it.
fmease
force-pushed
the
simplify-gen-param-default-users
branch
from
29551af to
d33e885
Compare
Member
Author
|
Ah, lol. I guess there's some merit in checking if it actually compiles and in reading the rest of a function body ^^. |
compiler-errors
approved these changes
Nov 11, 2024
fmease
force-pushed
the
simplify-gen-param-default-users
branch
from
d33e885 to
c247dec
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
bors
added
S-waiting-on-bors
This comment was marked as outdated.
This comment was marked as outdated.
bors
added
S-waiting-on-author
fmease
force-pushed
the
simplify-gen-param-default-users
branch
from
c247dec to
d0ddba3
Compare
Member
Author
|
@bors r=compiler-errors |
Collaborator
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 11, 2024
…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
…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
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 12, 2024
Rollup merge of rust-lang#132912 - fmease:simplify-gen-param-default-users, r=compiler-errors Simplify some places that deal with generic parameter defaults
| 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) { |
Contributor
There was a problem hiding this comment.
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.
Member
Author
There was a problem hiding this comment.
How do you feel about renaming it to default?
Member
Author
There was a problem hiding this comment.
Alternatively, default_term (cc, {ast, hir}::Term) or default_arg?
Contributor
There was a problem hiding this comment.
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.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.