Repository navigation
[RDataFrame] Do not transform #var inside string literals (fixes #23790) - #23800
Closed
muntasir-396 wants to merge 1 commit into
Closed
muntasir-396 wants to merge 1 commit into
muntasir-396 wants to merge 1 commit into
Conversation
…-project#23790) In RDataFrame expressions, #var is an alias for R_rdf_sizeof_var. Previously, a global regex replacement was applied over the entire expression, erroneously modifying '#' characters inside string literals (e.g. "#a" -> "R_rdf_sizeof_a"). This change introduces ReplaceHashVarOutsideLiterals, which skips string literals, raw string literals, character literals, and comments when expanding #var aliases. Added regression test ShortSyntaxRespectsStringLiterals in dataframe_interface.cxx.
Member
|
Dear @muntasir-396 , Thanks for showing interest in the project and submitting this PR! I believe the proposed changes introduce more code than necessary, I proposed a different approach that should supersede this PR at #23804 |
Contributor
Author
|
Thank you @vepadulano sir, for taking the time to review my PR and for suggesting an alternative approach. I appreciate your feedback. I’ll review PR #23804 to better understand the approach and learn from it. Thanks again for your guidance! |
Member
|
@muntasir-396 just FYI I added back your test which is a more direct check of the original issue to #23804 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #23790.
Problem
In RDataFrame expressions,
#varis an alias forR_rdf_sizeof_var.Previously, a global regex substitution was run over the raw expression string, modifying
#characters inside string literals (e.g.printString("#a")turned intoprintString("R_rdf_sizeof_a")).Solution
ReplaceHashVarOutsideLiteralsinRDFInterfaceUtils.cxxwhich scans the expression for string literals ("..."), raw string literals (R"(...)"), character literals ('...'), and comments, and only performs the#varalias replacement on tokens outside literals.ShortSyntaxRespectsStringLiteralsindataframe_interface.cxx.Checklist
dataframe_interface.cxx