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

Not compiling after adding the GAT Self: 'a bounds #90808

Closed
vandenheuvel opened this issue Nov 11, 2021 · 8 comments
Closed

Not compiling after adding the GAT Self: 'a bounds #90808

vandenheuvel opened this issue Nov 11, 2021 · 8 comments
Labels
A-GATs Area: Generic associated types (GATs) C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs GATs-triaged Issues using the `generic_associated_types` feature that have been triaged ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@vandenheuvel
Copy link
Contributor

vandenheuvel commented Nov 11, 2021

After #89970 was merged, project relp no longer compiled as expected: I needed to add a few Self: 'a bounds.

After I did just that, I was left with the following error:

error[E0477]: the type `RemoveRows<'_, MP>` does not fulfill the required lifetime
  --> src/algorithm/two_phase/mod.rs:57:21
   |
57 |                     phase_two::primal::<_, _, SteepestDescentAlongObjective<_>>(&mut non_artificial)
   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0477`.

After I made the following change:

-pub fn primal<IM, MP, PR>(
-    tableau: &mut Tableau<IM, NonArtificial<MP>>,
+pub fn primal<'provider, IM, MP, PR>(
+    tableau: &mut Tableau<IM, NonArtificial<'provider, MP>>,
 ) -> OptimizationResult<IM::F>
 where
-    IM: InverseMaintener<F: im_ops::FieldHR + im_ops::Column<<MP::Column as Column>::F>>,
-    for<'r> IM::F: im_ops::Cost<MP::Cost<'r>>,
+    IM: InverseMaintener<F:
+        im_ops::FieldHR +
+        im_ops::Column<<MP::Column as Column>::F> +
+        im_ops::Cost<MP::Cost<'provider>> +
+    >,

cargo build succeeded.

The core of this bug report is the question why the extra change above could have been necessary, as #89970 should not have influenced that.

Summarized from a private conversation with @jackh726.

@vandenheuvel vandenheuvel added the C-bug Category: This is a bug. label Nov 11, 2021
@jackh726 jackh726 added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Nov 11, 2021
@jackh726
Copy link
Member

@rustbot ping cleanup

It would be really nice to get a minimal test for this. The intent for this lint was that in theory, existing code shouldn't break. So, either we need to update the outlives lint code to not error here or decide that this pattern isn't correct and we should figure out the correct fix and make sure it's documented.

@rustbot rustbot added the ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections label Nov 11, 2021
@rustbot
Copy link
Collaborator

rustbot commented Nov 11, 2021

Hey Cleanup Crew ICE-breakers! This bug has been identified as a good
"Cleanup ICE-breaking candidate". In case it's useful, here are some
instructions for tackling these sorts of bugs. Maybe take a look?
Thanks! <3

cc @AminArria @ayazhafiz @camelid @chrissimpkins @contrun @elshize @h-michael @HallerPatrick @hdhoang @hellow554 @henryboisdequin @imtsuki @JamesPatrickGill @kanru @KarlK90 @matheus-consoli @mental32 @nmccarty @Noah-Kennedy @pard68 @PeytonT @pierreN @Redblueflame @RobbieClarken @RobertoSnap @robjtede @SarthakSingh31 @shekohex @sinato @smmalis37 @steffahn @Stupremee @tamuhey @turboladen @woshilapin @yerke

@jackh726
Copy link
Member

For clarification, the most ideal MCVE would be of the code at this commit.

@jackh726 jackh726 added the F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs label Nov 11, 2021
@camelid camelid added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 11, 2021
@vandenheuvel
Copy link
Contributor Author

vandenheuvel commented Nov 12, 2021

This issue might be related.

@vandenheuvel
Copy link
Contributor Author

vandenheuvel commented Nov 12, 2021

I am almost certain that this bug has to do with this line:

-    type Cost<'a> = MP::Cost<'a>;
+    type Cost<'a> where Self: 'a = MP::Cost<'a>;

The problem does not occur when the following change is applied before the where Self: 'a bound is added:

-    type Cost<'a> = MP::Cost<'a>;
+    type Cost<'a> = MP::Cost<'provider>;

On this branch, I applied the commits of this branch (where the original issue occured) in a different order. After starting with the commit above, I remove a few ugly HRTB's in an intermediate commit. After that, adding the Self: 'a clauses works as expected.

@jackh726
Copy link
Member

@vandenheuvel were you able to fix your project and also include the required bounds?

@jackh726
Copy link
Member

jackh726 commented Feb 7, 2022

GATs issue triage: not blocking. It's really not clear what an actionable path forward here is. It would be nice to know what exactly led to these problems, but I can't see anything that changes the outlives lint rules. We also have the flexibility in the future to make the outlives lint more lenient.

@jackh726 jackh726 added the GATs-triaged Issues using the `generic_associated_types` feature that have been triaged label Feb 7, 2022
@jackh726
Copy link
Member

jackh726 commented Sep 8, 2022

I'm going to go ahead and close this. As said in previous comment, not sure what is actionable here.

@jackh726 jackh726 closed this as completed Sep 8, 2022
@fmease fmease added the A-GATs Area: Generic associated types (GATs) label Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-GATs Area: Generic associated types (GATs) C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs GATs-triaged Issues using the `generic_associated_types` feature that have been triaged ICEBreaker-Cleanup-Crew Helping to "clean up" bugs with minimal examples and bisections T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants