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

set unused variable to nil after .forget #53

Open
proppy opened this issue Mar 30, 2023 · 5 comments
Open

set unused variable to nil after .forget #53

proppy opened this issue Mar 30, 2023 · 5 comments

Comments

@proppy
Copy link
Member

proppy commented Mar 30, 2023

https://www.klayout.de/doc/about/drc_ref_layer.html#h2-1260 recommends the following pattern after .forget'ing a variable:

l = ... # compute some layer
...
# once you're done with l:
l.forget
l = nil

we should make sure we follow it.

@atorkmabrains
Copy link
Collaborator

atorkmabrains commented Mar 30, 2023

@proppy klayout already sets the layer to nil when you call forget. And I believe we have demonstrated this in one of the PRs. This is just redundancy which I think is not necessary.

Here is the link to klayout forget code:
https://github.com/KLayout/klayout/blob/c931a08ec08bb626b28d3009e015a0964936b012/src/drc/drc/built-in-macros/_drc_layer.rb#L265

@proppy
Copy link
Member Author

proppy commented Mar 30, 2023

Then maybe we should submit a PR to klayout docstring https://github.com/KLayout/klayout/blob/c931a08ec08bb626b28d3009e015a0964936b012/src/drc/drc/built-in-macros/_drc_layer.rb#L253-L263 to stop advertising this as a best practice?

/fyi @klayoutmatthias

@atorkmabrains
Copy link
Collaborator

@proppy I think you might need to open that on Klayout. As I think forget method actually set the layer to nil as we have discussed.

@klayoutmatthias
Copy link

That is easy to change :) ... my reasoning actually was that without setting the layer to nil, you will find the layer variable still holding a valid object which however is dead. Internally of course the expensive payload object is set to nil already. I thought that might be considered as a kind of violation of basic OO contracts. But it's a matter of taste.

Ideally, setting the variable simply to nil would make the object forget, but Ruby is not like that because GC may be delayed.

@atorkmabrains
Copy link
Collaborator

atorkmabrains commented Apr 9, 2023

Thanks @klayoutmatthias

@proppy I believe we can close this at our end. Isn't that correct?

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

No branches or pull requests

3 participants