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

Casting self to a trait inside a default method is unsound #787

Open
yorickpeterse opened this issue Dec 16, 2024 · 0 comments
Open

Casting self to a trait inside a default method is unsound #787

yorickpeterse opened this issue Dec 16, 2024 · 0 comments
Labels
bug Defects, unintended behaviour, etc compiler Changes related to the compiler std Changes related to the standard library

Comments

@yorickpeterse
Copy link
Collaborator

Please describe the bug

When defining a default method in a trait, we currently allow one to cast or move self into a trait. For example:

trait TraitA {}

trait TraitB: TraitA {
  fn invalid1 {
    self as ref TraitA
  }

  fn invalid2 {
    let a: ref TraitA = self
  }
}

class A {}

impl TraitA for A {}

impl TraitB for A {}

This is unsound if the trait is implemented for a type which can't be cast to a trait, such as Int or an inline type as introduced per 8ca46bf and #783.

The mentioned PR introduces a self_type field on the TraitInstance structure, which we can use to detect if some type is self or not. This in turn can be used to disallow such casts. For example:

diff --git a/std/fixtures/diagnostics/cast_self_to_trait.inko b/std/fixtures/diagnostics/cast_self_to_trait.inko
new file mode 100644
index 00000000..0686be5e
--- /dev/null
+++ b/std/fixtures/diagnostics/cast_self_to_trait.inko
@@ -0,0 +1,23 @@
+fn example[T: TraitA](value: ref T) {}
+
+trait TraitA {}
+
+trait TraitB: TraitA {
+  fn invalid1 {
+    self as ref TraitA
+  }
+
+  fn invalid2 {
+    let a: ref TraitA = self
+  }
+
+  fn valid {
+    example(self)
+  }
+}
+
+class A {}
+
+impl TraitA for A {}
+
+impl TraitB for A {}
diff --git a/types/src/check.rs b/types/src/check.rs
index ecf75e04..f858a090 100644
--- a/types/src/check.rs
+++ b/types/src/check.rs
@@ -747,14 +747,22 @@ impl<'a> TypeChecker<'a> {
             },
             TypeId::TraitInstance(lhs) => match right_id {
                 TypeId::TraitInstance(rhs) => {
+                    if lhs.self_type {
+                        return false;
+                    }
+
                     self.check_traits(lhs, rhs, env, rules)
                 }
                 TypeId::TypeParameter(_) if rules.kind.is_cast() => false,
                 TypeId::TypeParameter(rhs) if rhs.is_copy(self.db) => false,
-                TypeId::TypeParameter(rhs) => rhs
-                    .requirements(self.db)
-                    .into_iter()
-                    .all(|req| self.check_traits(lhs, req, env, rules)),
+                TypeId::TypeParameter(rhs) => {
+                    let mut lhs = lhs;
+
+                    lhs.self_type = false;
+                    rhs.requirements(self.db)
+                        .into_iter()
+                        .all(|req| self.check_traits(lhs, req, env, rules))
+                }
                 _ => false,
             },
             TypeId::TypeParameter(lhs) => match right_id {

Applying the above diff introduces another problem: std.iter.Peekable is implemented such that it expects a boxed Iter value as its input. This means that std.iter.Iter.peekable becomes invalid with the above diff applied, with no suitable workaround. To resolve that, we also need to re-introduce Self as a type such that peekable() can be implemented as fn pub move peekable -> Peekable[T, Self] with the Peekable signature changed to Peekable[T, I: mut + Iter[T]].

Blocked by

Operating system

Fedora

Inko version

main

@yorickpeterse yorickpeterse added bug Defects, unintended behaviour, etc compiler Changes related to the compiler std Changes related to the standard library labels Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defects, unintended behaviour, etc compiler Changes related to the compiler std Changes related to the standard library
Projects
None yet
Development

No branches or pull requests

1 participant