-
-
Notifications
You must be signed in to change notification settings - Fork 706
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
fix issue 18470 - std.algorithm.splitter has frame access problems for custom preds #6522
Conversation
|
Thanks for your pull request and interest in making D better, @bbasile! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + phobos#6522" |
std/algorithm/iteration.d
Outdated
| @@ -3781,8 +3781,16 @@ if (is(typeof(binaryFun!pred(r.front, s)) : bool) | |||
| { | |||
| static size_t lastIndexOf(Range haystack, Separator needle) | |||
| { | |||
| // issue 18470 : we need to "copy" the custom pred. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"recontextualize" rather than "copy".
|
This looks more like a compiler bug at first glance. Surely fixing it there would be preferable? |
|
This is not a compiler bug, the problem is that the range is static. So there's another way to solve the issue. |
std/algorithm/iteration.d
Outdated
| @@ -3758,7 +3758,7 @@ if (is(typeof(binaryFun!pred(r.front, s)) : bool) | |||
| import std.algorithm.searching : find; | |||
| import std.conv : unsigned; | |||
|
|
|||
| static struct Result | |||
| struct Result | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove extra space
std/algorithm/iteration.d
Outdated
| @safe unittest // issue 18470 | ||
| { | ||
| import std.algorithm; | ||
| import std.range; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to specific function imports
std/algorithm/iteration.d
Outdated
|
|
||
| @safe unittest // issue 18470 | ||
| { | ||
| import std.algorithm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a viable and simple solution with the only downside being a larger stack size.
|
Addressed review comments and squashed to delete the first solution. |
No description provided.