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

What to do with maybe-invalid indexes for primSubstringFrom #73

Open
ltratt opened this issue Apr 16, 2021 · 3 comments
Open

What to do with maybe-invalid indexes for primSubstringFrom #73

ltratt opened this issue Apr 16, 2021 · 3 comments

Comments

@ltratt
Copy link

ltratt commented Apr 16, 2021

String substringFrom carefully checks bounds and throws an error if they're invalid. However, other functions in String.som call primSubstringFrom directly and sometimes seem to pass start==0, end==0, and end<start. What should primSubstringFrom do in such cases? Throw an error? Return the empty string? Or ... ?

@smarr
Copy link
Member

smarr commented Apr 16, 2021

On my phone.
First though: this sounds like a bug in the using functions and should be fixed there. You got perhaps a test?

Second thought: I never liked this primSubstring thing. It's one of those Smalltalk-isms that break easily because we don't have private methods.

I think the primitive should do what the method does. And be renamed. To replace the method.
Drawback of this is that we can't look at the Smalltalk to see what happens on error

@ltratt
Copy link
Author

ltratt commented Apr 16, 2021

OK, so the bug is that String split: borks if you pass it a string starting with the split string. e.g. '/a/b' split: '/' leads to an index exception in primSubstringFrom. [I found this because I was using absolute paths to pass to SomSom!]

@ltratt
Copy link
Author

ltratt commented Apr 16, 2021

[There may be other methods that call primSubstringFrom incorrectly, but this was the one I've found so far. Looking at som-java, it just throws a Java out of bounds exception in such cases. I should probably convert that into a yksom exception.]

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

No branches or pull requests

2 participants