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

Make Attribute.get_value rw. Fixes #2521 #2526

Closed
wants to merge 1 commit into from
Closed

Conversation

FCO
Copy link
Member

@FCO FCO commented Dec 8, 2018

No description provided.

@lizmat
Copy link
Contributor

lizmat commented Dec 8, 2018

Currently, you cannot change the value of the attribute with get_value, only with set_value. This change would allow a container to leak out, even if the attribute was not marked is rw. And with this change that would only work for non-native (Opaque) attributes.

It is true that set_value doesn't respect the is rw setting on the attribute either.

In any case, it feels to me that either it shouldn't matter whether the attribute is a native attribute or not. Or that this PR should be rejected because if you want to be that low-level, you should use set_value if you want to set the value of an Attribute in an object that way.

@lizmat
Copy link
Contributor

lizmat commented Dec 8, 2018

After some more thought today, I think we can consider Attribute.get_value and Attribute.set_value sufficiently low-level to take all of the safeties off. Working in that now.

@lizmat
Copy link
Contributor

lizmat commented Dec 8, 2018

Thanks for the pointers. Fixed it more extensively with a5411e4

@lizmat lizmat closed this Dec 8, 2018
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.

None yet

2 participants