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

[develop] String:findRegexp handles empty results incorrectly #4239

Closed
jamshark70 opened this issue Jan 9, 2019 · 2 comments · Fixed by #4241
Closed

[develop] String:findRegexp handles empty results incorrectly #4239

jamshark70 opened this issue Jan 9, 2019 · 2 comments · Fixed by #4241
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"

Comments

@jamshark70
Copy link
Contributor

Environment

  • Your SuperCollider version: develop branch (0685eb1)
  • Your operating system and version: Ubuntu Studio 16.04

Steps to reproduce (for bugs)

// 3.10:
"abc".findRegexp("def");
-> [  ]

// current develop branch:
"abc".findRegexp("def");
-> def

Expected Behavior

If String:findRegexp doesn't find any matches, it should return an empty array.

Current Behavior

Between 3.10 and develop, the behavior changed so that an empty result returns the regular expression as a string.

https://github.com/supercollider/supercollider/blame/develop/lang/LangPrimSource/PyrStringPrim.cpp#L416

^^ I'm guessing this is the issue. Right above this line, there is something new to advance the stack (comment says "to avoid overwriting receiver"). But if the match result is empty, now we simply return without un-advancing the stack -- so the returned result is wrong.

I suppose the fix is trivial, but I am not good with GC details.

User impact: I use findRegexp extensively in ddwChucklib-livecode. It's impossible to evaluate any cll statements in the current develop branch.

@jamshark70 jamshark70 added bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library" labels Jan 9, 2019
@mossheim
Copy link
Contributor

mossheim commented Jan 9, 2019

thanks for reporting this @jamshark70 . it looks like the solution is just to remove that line altogether since the loop will be skipped anyway if match_count == 0. (then add a basic unit test.)

btw, i gave #4192 another look to make sure no similar errors were introduced; the other changes are fine as they don't involve early exits. i should have caught that in review, my apologies.

[cc @muellmusik ]

@jamshark70
Copy link
Contributor Author

Tried it and it works. #4241 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants