Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test that we do not support higher-ranked regions in opaque type inference #121386

Merged
merged 6 commits into from
Feb 22, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 21, 2024

We already do all the right checks in check_opaque_type_parameter_valid, and we have done so since at least 2 years.

I collected the tests from #116935 and #100503 and added some more

cc #96146

r? @lcnr

@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 Feb 21, 2024
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

some rustc improvements i thought of while looking at the tests.
please either implement them in this PR or open separate issues for them.

separately: we should document somewhere (at least in the ATPIT stabilization report, cc @traviscross ), that higher ranked opaque types mentioning higher-ranked lifetimes are not supported

LL | const FOO: Foo = Struct;
| ^^^^^^

error: concrete type differs from previous defining opaque type use
Copy link
Contributor

@lcnr lcnr Feb 22, 2024

Choose a reason for hiding this comment

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

can you yeet this error if there's a {type error} in the expected found? 😁

Copy link
Contributor

@lcnr lcnr 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 nit

@@ -845,26 +845,27 @@ impl<'tcx> OpaqueHiddenType<'tcx> {
other: &Self,
opaque_def_id: LocalDefId,
tcx: TyCtxt<'tcx>,
) -> DiagnosticBuilder<'tcx> {
) -> Result<DiagnosticBuilder<'tcx>, ErrorGuaranteed> {
Copy link
Contributor

Choose a reason for hiding this comment

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

vibe: rename this method to fn build_mismatch_error or sth? because a method called report returing Ok(DiagnosticsBuilder) feels off to me

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 22, 2024

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Feb 22, 2024

📌 Commit e4622e0 has been approved by lcnr

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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 22, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 22, 2024
…r=lcnr

test that we do not support higher-ranked regions in opaque type inference

We already do all the right checks in `check_opaque_type_parameter_valid`, and we have done so since at least 2 years.

I collected the tests from rust-lang#116935 and rust-lang#100503 and added some more

cc rust-lang#96146

r? `@lcnr`
// Regression test for #97099.
// This was an ICE because `impl Sized` captures the lifetime 'a.

// check-fail
Copy link
Member

Choose a reason for hiding this comment

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

@oli-obk Oli did you forgor to update these UI test directives to use //@?

Copy link
Member

@lqd lqd Feb 22, 2024

Choose a reason for hiding this comment

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

check-fail should be the default for these tests anyways, so we could remove the directives rather than use //@

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 22, 2024

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Feb 22, 2024

📌 Commit 1efb747 has been approved by lcnr

It is now in the queue for this repository.

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#120598 (No need to `validate_alias_bound_self_from_param_env` in `assemble_alias_bound_candidates`)
 - rust-lang#121386 (test that we do not support higher-ranked regions in opaque type inference)
 - rust-lang#121393 (match lowering: Introduce a `TestCase` enum to replace most matching on `PatKind`)
 - rust-lang#121401 (Fix typo in serialized.rs)
 - rust-lang#121427 (Fix panic when compiling `Rocket`.)
 - rust-lang#121439 (Fix typo in metadata.rs doc comment)
 - rust-lang#121441 (`DefId`  to `LocalDefId`)
 - rust-lang#121452 (Add new maintainers to nto-qnx.md)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 379ef9b into rust-lang:master Feb 22, 2024
@rustbot rustbot added this to the 1.78.0 milestone Feb 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 22, 2024
Rollup merge of rust-lang#121386 - oli-obk:no_higher_ranked_opaques, r=lcnr

test that we do not support higher-ranked regions in opaque type inference

We already do all the right checks in `check_opaque_type_parameter_valid`, and we have done so since at least 2 years.

I collected the tests from rust-lang#116935 and rust-lang#100503 and added some more

cc rust-lang#96146

r? `@lcnr`
# for free to join this conversation on GitHub. Already have an account? # 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.

7 participants