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

Change the algorithm in validate_terminates #1220

Merged
merged 6 commits into from
Nov 13, 2024

Conversation

arnaudgolfouse
Copy link
Collaborator

@jhjourdan this is what we talked about.

The new algorithm is stricter:

trait Foo {
    fn foo1();
    fn foo2();
}

impl Foo for i32 {
    fn foo1() {}
    fn foo2() { g() }
}

fn f<T: Foo>() { <T as Foo>::foo1() }
fn g() { f::<i32>(); }

is now rejected, saying that f might call i32::foo2 because of the T: Foo bound.

@arnaudgolfouse arnaudgolfouse force-pushed the fix-validate-terminates branch 2 times, most recently from 0a7866a to 67ac01f Compare November 12, 2024 14:18
@arnaudgolfouse arnaudgolfouse marked this pull request as ready for review November 12, 2024 14:18
@arnaudgolfouse arnaudgolfouse force-pushed the fix-validate-terminates branch from 67ac01f to d9a63c0 Compare November 12, 2024 16:47
@arnaudgolfouse
Copy link
Collaborator Author

This is technically incorrect, but there should be a path to fix that (see #1232). Also usage of rustc's internal APIs is better, avoiding some crashes.

@jhjourdan
Copy link
Collaborator

We discussed this with @arnaudgolfouse. We think that this is an improvement over the current state, even though there are some remaining problems. Still, we need this to be merged before #1228, so I'm going to merge this, and postpone solving #1232 later.

@jhjourdan jhjourdan merged commit 716d582 into creusot-rs:master Nov 13, 2024
4 checks passed
@arnaudgolfouse arnaudgolfouse deleted the fix-validate-terminates branch December 5, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants