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

The ownership of a nested field used inside a mutable method is incorrect #642

Closed
yorickpeterse opened this issue Nov 10, 2023 · 1 comment
Labels
bug Defects, unintended behaviour, etc compiler Changes related to the compiler
Milestone

Comments

@yorickpeterse
Copy link
Collaborator

yorickpeterse commented Nov 10, 2023

Please describe the bug

For expressions such as @some_field.nested_field, where nested_field is typed as T, if the surrounding method is fn mut, the return type is T instead of mut T. However, if the surrounding method is just fn the return type is (as expected) ref T. Thus, it seems that for fn mut we're no longer applying the correct ownership to nested fields.

Please list the exact steps necessary to reproduce the bug

Run this:

class Lexer[T] {
  let @input: T
}

class Parser[T] {
  let @lexer: Lexer[T]

  fn mut example {
    @lexer.input.TEST
  }
}

class async Main {
  fn async main {}
}

This produces:

/var/home/yorickpeterse/homes/arch/Downloads/test.inko:9:5 error(invalid-symbol): the method 'TEST' isn't defined for type 'T'

What it instead should produce is:

/var/home/yorickpeterse/homes/arch/Downloads/test.inko:9:5 error(invalid-symbol): the method 'TEST' isn't defined for type 'mut T'

Operating system

Arch Linux

Inko version

0.13.1

Rust version

1.73.0

Related issues

@yorickpeterse yorickpeterse added accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers bug Defects, unintended behaviour, etc compiler Changes related to the compiler labels Nov 10, 2023
@yorickpeterse
Copy link
Collaborator Author

yorickpeterse commented Nov 11, 2023

The underlying problem is this:

In the above example, T isn't defined as T: mut, so when we end up calling TypeRef::as_mut() (when type-checking field access), we run into this code:

inko/types/src/lib.rs

Lines 3622 to 3626 in b3f73c5

if pid.is_mutable(db) {
TypeRef::Mut(id)
} else {
self
}

Because of the lack of T: mut, we then return the type as-is.

This poses a new question: inside an fn mut, what should the type of a type parameter T be if it doesn't specify the mut requirement? If we return mut T we may allow mutations, which may be unsound (e.g. if T is a ref Something at runtime). If we return a ref T instead this could potentially be confusing, but at least it would be sound.

This is also related to #618, and specifically my thoughts in #618 (comment).

@yorickpeterse yorickpeterse removed the accepting contributions Issues that are suitable to be worked on by anybody, not just maintainers label Nov 11, 2023
@yorickpeterse yorickpeterse added this to the 0.13.2 milestone Nov 11, 2023
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
Projects
None yet
Development

No branches or pull requests

1 participant