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

regression: concrete type differs from previous defining opaque type use #115781

Closed
Mark-Simulacrum opened this issue Sep 12, 2023 · 5 comments
Closed
Assignees
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Milestone

Comments

@Mark-Simulacrum
Copy link
Member

[INFO] [stdout] error: concrete type differs from previous defining opaque type use
[INFO] [stdout]    --> src/execution.rs:222:13
[INFO] [stdout]     |
[INFO] [stdout] 222 |             impl Iterator<Item = Node<'tree>> + 's,
[INFO] [stdout]     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[INFO] [stdout]     |             |
[INFO] [stdout]     |             expected `impl Iterator<Item = tree_sitter::Node<'tree>> + 'tree`, got `impl Iterator<Item = tree_sitter::Node<'s>> + 's`
[INFO] [stdout]     |             this expression supplies two conflicting concrete types for the same opaque type

https://crater-reports.s3.amazonaws.com/beta-1.73-1.2/beta-2023-09-10/reg/tree-sitter-graph-0.11.0/log.txt

@Mark-Simulacrum Mark-Simulacrum added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Sep 12, 2023
@Mark-Simulacrum Mark-Simulacrum added this to the 1.73.0 milestone Sep 12, 2023
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 12, 2023
@Mark-Simulacrum Mark-Simulacrum removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 12, 2023
@Noratrieb Noratrieb added A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Sep 12, 2023
@apiraino
Copy link
Contributor

this regressed in 0dd5730:

The exact commit is not available anymore (0ae3a8e2ad80d9e375cb8f391bfb0e5e57d10515) but by reading the tests of #113578 I think this is where it originated and (if I understand) is the intended behaviour i.e. introduce an error to fix an unsoundness (cc @oli-obk )

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 14, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Sep 14, 2023

hmm... that PR was only supposed to fix a type_alias_impl_trait unsoundness, not have any effect on stable crates and RPIT. I'll investigate

@oli-obk oli-obk self-assigned this Sep 14, 2023
@oli-obk oli-obk added the I-types-nominated Nominated for discussion during a types team meeting. label Sep 14, 2023
@oli-obk oli-obk removed the I-types-nominated Nominated for discussion during a types team meeting. label Sep 14, 2023
@apiraino
Copy link
Contributor

@oli-obk my apologies! I mistyped the PR that I thought was the source of the regression. Commit 0ae3a8e2ad80d9e375cb8f391bfb0e5e57d10515 leads to #113661 as it can be inferred by the perf build results of the rollup merge in comment (a trick I've learned today).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 14, 2023
… r=jackh726

Paper over an accidental regression

r? types

cc rust-lang#115781 (do not close issue until beta backport has been performed)

The PR reasons are explained with comments in the source.

In order to keep the diff simple, this PR effectively reverts rust-lang#113661, but only for RPITs. I will submit a follow up PR that fixes this correctly instead of just disabling the newly added check for RPITs. This PR should be significantly easier to review for beta backport
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 14, 2023
Rollup merge of rust-lang#115844 - oli-obk:opaque_lifetime_ambiguity, r=jackh726

Paper over an accidental regression

r? types

cc rust-lang#115781 (do not close issue until beta backport has been performed)

The PR reasons are explained with comments in the source.

In order to keep the diff simple, this PR effectively reverts rust-lang#113661, but only for RPITs. I will submit a follow up PR that fixes this correctly instead of just disabling the newly added check for RPITs. This PR should be significantly easier to review for beta backport
@oli-obk
Copy link
Contributor

oli-obk commented Sep 25, 2023

beta backported a fix in #116044

@oli-obk oli-obk closed this as completed Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. P-medium Medium priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants