Skip to content

Strengthen check for inference type loops #2448

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

swernli
Copy link
Collaborator

@swernli swernli commented May 22, 2025

This change updats the check we have in type inference to ensure an inference type is not bound to another type that includes itself. The previous check worked for simple cases where the target type directly contained the inference type, but fails to catch recursion where the target type contained inference types that indicrectly pointed to the original inference type through the current type inference solution. With this check in place, we can now explicitly give an error when a recursive type is detected.
Fixes #2426

This change updats the check we have in type inference to ensure an inference type is not bound to another type that includes itself. The previous check worked for simple cases where the target type directly contained the inference type, but fails to catch recursion where the target type contained inference types that indicrectly pointed to the original inference type through the current type inference solution. With this check in place, we can now explicitly give an error when a recursive type is detected.
Fixes #2426
@swernli swernli requested review from idavis and minestarks as code owners May 22, 2025 18:28
@@ -2230,5 +2230,43 @@ mod given_interpreter {
"#]],
);
}

#[test]
fn fuzz_2426() {
Copy link
Member

Choose a reason for hiding this comment

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

There's got to be a better name. recursive_type_constraint maybe?

Comment on lines +2245 to +2253
match Interpreter::new(
sources,
PackageType::Lib,
TargetCapabilityFlags::all(),
LanguageFeatures::default(),
store,
&[(std_id, None)],
) {
Ok(_) => panic!("interpreter should fail with error"),
Copy link
Member

Choose a reason for hiding this comment

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

.expect_err?

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.

Fuzz: "==5070== ERROR: libFuzzer: out-of-memory (used: 4204Mb; limit: 4096Mb)" (ubuntu-latest)
2 participants