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 $xml_find_function_calls() land on parent::expr instead #2715

Merged
merged 6 commits into from
Feb 11, 2025

Conversation

MichaelChirico
Copy link
Collaborator

@MichaelChirico MichaelChirico commented Feb 6, 2025

Closes #2431.

@MichaelChirico
Copy link
Collaborator Author

@AshesITR / @IndrajeetPatil PTAL at this today if you can. I need to submit an update to CRAN by tomorrow.

If you prefer, I can just put a small patch on CRAN to meet the deadline & allow a little longer for review here.

@AshesITR
Copy link
Collaborator

The change is okay for me on the grounds that we could remove many more parent::expr than we had to add SYMBOL_FUNCTION_CALL.

Have you checked which of self::expr and self::* is faster? I assume it's self::* which would justify its usage despite being slightly less clear IMO.

@MichaelChirico
Copy link
Collaborator Author

I'm 90% sure we've benchmarked * to be faster, but it's worth double-checking. Not worth pausing the PR over, though: #2718

@MichaelChirico MichaelChirico merged commit 39f7a59 into main Feb 11, 2025
19 checks passed
@MichaelChirico MichaelChirico deleted the find-function-parent branch February 11, 2025 17:42
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.

Should xml_find_function_calls() land on the parent expr instead?
2 participants