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

Unable to get nested mut VTag from mut VList #2689

Closed
XX opened this issue May 17, 2022 · 3 comments · Fixed by #2692
Closed

Unable to get nested mut VTag from mut VList #2689

XX opened this issue May 17, 2022 · 3 comments · Fixed by #2692
Labels

Comments

@XX
Copy link
Contributor

XX commented May 17, 2022

I have the extension methods for VTag like this:

fn find_child_contains_class_mut(&mut self, class: &str) -> Option<&mut VTag>;
fn find_child_contains_class_recursively_mut(&mut self, class: &str) -> Option<&mut VTag>;

They were broken after using ChildrenMut as return type of the VList::children_mut: #2673. Now I have lost the ability to use these methods in API of my libs. ((

How to fix it?

@XX XX added the bug label May 17, 2022
@WorldSEnder
Copy link
Member

WorldSEnder commented May 18, 2022

ChildrenMut has a DerefMut implementation to Vec<VNode> that you can use to access the nodes of the list directly. Is there any problem you're facing with that approach?

@XX
Copy link
Contributor Author

XX commented May 18, 2022

@WorldSEnder The problem is that the ChildrenMut object is dropped at the end of the method, so it's impossible to return the VTag reference. And for the recursive search, it's impossible to return the final ChildrenMut object also.

@WorldSEnder
Copy link
Member

Right, that problem is akin to only getting Ref from a RefCell when borrowing, because it has to restore some internal invariants when unborrowing. In our case, the invariant it re-checks is fully_keyed, which solves the problem in #2654 (no manual check needed any longer).

I see a potential solution: delay checking fully_keyed for vlists that get borrowed mutably until they get mounted.

2 small concerns about that, for completeness, I still think it's a good solution:

  • traverses a potentially long list at mount time, and the performance hit is hard to track back to where the VList was constructed.
  • need to ensure that known-good lists from the macro still optimize with a compile time computed fully_keyed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants