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

internal anyNamed no longer returns false? #4424

Closed
jangorecki opened this issue May 4, 2020 · 1 comment · Fixed by #4655
Closed

internal anyNamed no longer returns false? #4424

jangorecki opened this issue May 4, 2020 · 1 comment · Fixed by #4655
Milestone

Comments

@jangorecki
Copy link
Member

jangorecki commented May 4, 2020

According to codecov the follow line is no longer visited:

return false;

Most likely the function may no longer return false (unless we have poor coverage of it), which is probably a result of switching codecov environment from R 3.6 to R 4.0. We might want to revisit use of anyNamed function.

@tlapak
Copy link
Contributor

tlapak commented May 5, 2020

Unfortunately, I don't have the time to implement and test a fix but I did have the time to look through the sources and was curious. Maybe this helps.

The function can still return false, I guess, just not when it's called on a non-empty list. But that's currently all that it's used for. Thing is that with reference counting turned on, MAYBE_NAMED checks the reference count and SET_VECTOR_ELT bumps up the reference count of the element it sticks into the list so that it's always at least one. With reference counting turned off, the named attribute doesn't get set. So you'd need to branch for the inner call only based on reference counting yes/no. Or rip out the test one way or another.

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

Successfully merging a pull request may close this issue.

3 participants