Skip to content

Deny defaults for higher-ranked generic parameters #119494

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
Merged
merged 1 commit into from
Jan 3, 2024
Merged

Deny defaults for higher-ranked generic parameters #119494

merged 1 commit into from
Jan 3, 2024

Conversation

fmease
Copy link
Member

@fmease fmease commented Jan 1, 2024

Fixes #119489 (incl. #119489 (comment)).
Partially reverts #119042.

cc @bvanjoi
r? @compiler-errors or compiler

@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 Jan 1, 2024
@fmease fmease added the F-non_lifetime_binders `#![feature(non_lifetime_binders)]` label Jan 1, 2024
fmease
fmease commented Jan 1, 2024
View reviewed changes
compiler/rustc_ast_lowering/messages.ftl Outdated
@@ -62,6 +60,9 @@ ast_lowering_generic_type_with_parentheses =
parenthesized type parameters may only be used with a `Fn` trait
.label = only `Fn` traits may use parentheses

ast_lowering_higher_ranked_gen_param_with_default =
higher-ranked generic parameters may not have defaults
Copy link
Member Author

@fmease fmease Jan 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Alternatively: defaults for {type|const|generic} parameters are not allowed in for<...> binders to mirror the diagnostic defaults for type parameters are only allowed in struct, enum, type, or trait definitions as well as some other diagnostics that use for<...> over higher-ranked.

Copy link
Member

Choose a reason for hiding this comment

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

sure, i do like that alternative wording better

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@fmease fmease force-pushed the deny-hr-param-defaults branch from be656cf to 196e3e4 Compare January 1, 2024 19:01
fmease
fmease commented Jan 1, 2024
View reviewed changes
compiler/rustc_ast_lowering/src/lib.rs
@@ -884,27 +883,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
binder: NodeId,
generic_params: &[GenericParam],
) -> &'hir [hir::GenericParam<'hir>] {
let mut generic_params: Vec<_> = generic_params
Copy link
Member Author

Choose a reason for hiding this comment

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

Moving this into lower_generic_param_kind since lower_lifetime_binder doesn't get called on HRTBs.

fmease
fmease commented Jan 1, 2024
View reviewed changes
tests/ui/traits/non_lifetime_binders/binder-defaults-118697.stderr Outdated
|
LL | type T = dyn for<V = A(&())> Fn(());
| ^ not found in this scope

error: default parameter is not allowed in this binder
--> $DIR/issue-118697.rs:4:22
error: higher-ranked generic parameters may not have defaults
Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the diagnostic message since I didn't find it clear and since we're now generally rejecting defaults for higher-ranked generic parameters.

compiler-errors
compiler-errors approved these changes Jan 1, 2024
View reviewed changes
compiler/rustc_ast_lowering/messages.ftl Outdated
@@ -62,6 +60,9 @@ ast_lowering_generic_type_with_parentheses =
parenthesized type parameters may only be used with a `Fn` trait
.label = only `Fn` traits may use parentheses

ast_lowering_higher_ranked_gen_param_with_default =
higher-ranked generic parameters may not have defaults
Copy link
Member

Choose a reason for hiding this comment

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

sure, i do like that alternative wording better

@compiler-errors
Copy link
Member

thanks @fmease, I should've probably reviewed the other PR more carefully.

r=me

@fmease fmease force-pushed the deny-hr-param-defaults branch from 196e3e4 to 3d0297a Compare January 1, 2024 20:59
@fmease
Copy link
Member Author

fmease commented Jan 1, 2024

Updated to use the alternative wording.
@bors r=compiler-errors rollup

@bors
Copy link
Collaborator

bors commented Jan 1, 2024

📌 Commit 3d0297a has been approved by compiler-errors

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jan 1, 2024

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

@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 Jan 1, 2024
fmease added a commit to fmease/rust that referenced this pull request Jan 2, 2024
@fmease
Rollup merge of rust-lang#119494 - fmease:deny-hr-param-defaults, r=c…
55dd7c5 
…ompiler-errors

Deny defaults for higher-ranked generic parameters

Fixes rust-lang#119489 (incl. rust-lang#119489 (comment)).
Partially reverts rust-lang#119042.

cc `@bvanjoi`
r? `@compiler-errors` or compiler
@fmease fmease mentioned this pull request Jan 2, 2024
Closed
fmease added a commit to fmease/rust that referenced this pull request Jan 3, 2024
@fmease
Rollup merge of rust-lang#119494 - fmease:deny-hr-param-defaults, r=c…
bb124aa 
…ompiler-errors

Deny defaults for higher-ranked generic parameters

Fixes rust-lang#119489 (incl. rust-lang#119489 (comment)).
Partially reverts rust-lang#119042.

cc ``@bvanjoi``
r? ``@compiler-errors`` or compiler
This was referenced Jan 3, 2024
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 3, 2024
@bors
Auto merge of rust-lang#119549 - fmease:rollup-jxvbfes, r=fmease
139fb22 
Rollup of 21 pull requests

Successful merges:

 - rust-lang#119086 (Query panic!() to useful diagnostic)
 - rust-lang#119239 (Remove unnecessary arm in `check_expr_yield`)
 - rust-lang#119298 (suppress change-tracker warnings in CI containers)
 - rust-lang#119319 (Document that File does not buffer reads/writes)
 - rust-lang#119434 (rc: Take *const T in is_dangling)
 - rust-lang#119444 (Rename `TyCtxt::is_closure` to `TyCtxt::is_closure_or_coroutine`)
 - rust-lang#119474 (Update tracking issue of naked_functions)
 - rust-lang#119476 (Pretty-print always-const trait predicates correctly)
 - rust-lang#119477 (rustdoc ui: adjust tooltip z-index to be above sidebar)
 - rust-lang#119479 (Remove two unused feature gates from rustc_query_impl)
 - rust-lang#119487 (Minor improvements in comment on `freshen.rs`)
 - rust-lang#119492 (Update books)
 - rust-lang#119494 (Deny defaults for higher-ranked generic parameters)
 - rust-lang#119498 (Update deadlinks of `strict_provenance` lints)
 - rust-lang#119505 (Don't synthesize host effect params for trait associated functions marked const)
 - rust-lang#119510 (Report I/O errors from rmeta encoding with emit_fatal)
 - rust-lang#119512 (Mark myself as back from leave)
 - rust-lang#119514 (coverage: Avoid a query stability hazard in `function_coverage_map`)
 - rust-lang#119523 (llvm: Allow `noundef` in codegen tests)
 - rust-lang#119534 (Update `thread_local` examples to use `local_key_cell_methods`)
 - rust-lang#119544 (Fix: Properly set vendor in i686-win7-windows-msvc target)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b5618bc into rust-lang:master Jan 3, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 3, 2024
@rust-timer
Unrolled build for rust-lang#119494
d88a0ad 
Rollup merge of rust-lang#119494 - fmease:deny-hr-param-defaults, r=compiler-errors

Deny defaults for higher-ranked generic parameters

Fixes rust-lang#119489 (incl. rust-lang#119489 (comment)).
Partially reverts rust-lang#119042.

cc ```@bvanjoi```
r? ```@compiler-errors``` or compiler
@fmease fmease deleted the deny-hr-param-defaults branch January 3, 2024 20:57
@fmease fmease mentioned this pull request Apr 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

Assignees

@compiler-errors compiler-errors

Labels
F-non_lifetime_binders `#![feature(non_lifetime_binders)]` 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.77.0
Development

Successfully merging this pull request may close these issues.

ICE: no entry found for key, expr.rs
5 participants
@fmease @rust-log-analyzer @compiler-errors @bors @rustbot