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

[Feature Request] returning values in __del__ overload #2822

Closed
1 task done
rd4com opened this issue May 25, 2024 · 2 comments
Closed
1 task done

[Feature Request] returning values in __del__ overload #2822

rd4com opened this issue May 25, 2024 · 2 comments
Labels
enhancement New feature or request mojo-repo Tag all issues with this label

Comments

@rd4com
Copy link
Contributor

rd4com commented May 25, 2024

Review Mojo's priorities

What is your request?

Thought about the Dict for a while,
it would be nice to be able to move a value out of __del__() (because it is a place where struct can be not whole).

Maybe a new taking del could help with return DictEntry.value^ ?

An example of what could be accomplished:

@destructor
fn __del2__(owned self)->V: 
    self.key^.__del__()
    return self.value^
var x = instance^.__del2__()

(Not sure about the decorator)

What is your motivation for this change?

Having DictEntry.value as an Optional could be the thing,
but since performance is a priority, maybe it would become slower ? (no benchmarks done)

The challenge is to be able to take the value field of a DictEntry as an owned value,

that way, users would not get a copy when doing Dict.pop("") and it could fix #2756 .

It is possible that it returns the value as a copy, because return entry_value.value^ is different than return entry_value^.value^.

 

Reference to #2756

Please have a look at the Dict.pop, it is weird that we are doing return entry_value.value^,

how can the value entry_value be destroyed if it is not whole ? (Documentation: field lifetimes)
This might be the source of the double free ?

Any other details?

No response

@rd4com rd4com added enhancement New feature or request mojo-repo Tag all issues with this label labels May 25, 2024
@JoeLoser
Copy link
Collaborator

JoeLoser commented May 25, 2024

@stumpOS @lattner do you have ideas here with respect to CheckLifetimes?

@lattner
Copy link
Collaborator

lattner commented Jun 16, 2024

You cannot make this specifically a 'del' but you can already make consuming methods, and they can return values. You have to explicitly disable the destructor of 'self' or else it will be called, which is the trick. This is equivalent to Rust's "mem::forget". We don't have syntax for this, but you can write it out manually with mlir magic:

  fn take_value(owned self) -> V:
      # don't run the destructor on 'self'
      __mlir_op.`lit.ownership.mark_destroyed`(__get_mvalue_as_litref(self))
     # Can now pillage the pieces of 'self' that I want.  Any unused subfields
     # will be individual destroyed.
     return self.some_part^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request mojo-repo Tag all issues with this label
Projects
None yet
Development

No branches or pull requests

3 participants