std.algorithm.searching: Fix a -dip1000 compilable issue#6246
std.algorithm.searching: Fix a -dip1000 compilable issue#6246WalterBright merged 1 commit intodlang:masterfrom carblue:dip1000_7
Conversation
|
Thanks for your pull request and interest in making D better, @carblue! 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 referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
| ptrdiff_t occurrence(ElementType!(Range) c) | ||
| ptrdiff_t occurrence(ElementType!(Range) c) scope | ||
| { | ||
| auto p = c in occ; |
There was a problem hiding this comment.
We should better leave this to inference as opHash or opEquals of the element type could possibly escape a reference to one of the keys of the occ hash table. Somewhat unlikely but still possible, e.g. some cache of expensive hashing.
There was a problem hiding this comment.
This sounds like rejection of PR, thus I will close it
There was a problem hiding this comment.
@MartinNowak PR reopened:
I didn't gratuitously add scope, because for the implicit this parameter, it doesn't get inferred currently, as this will show. Temporarily apply these changes:
diff --git a/std/algorithm/searching.d b/std/algorithm/searching.d
index 42e128f..084bfc0 100644
--- a/std/algorithm/searching.d
+++ b/std/algorithm/searching.d
@@ -360,7 +360,7 @@ public:
}
///
- Range beFound(Range haystack)
+ Range beFound(Range haystack) @safe
{
import std.algorithm.comparison : max;
@@ -1839,7 +1839,7 @@ if (isInputRange!InputRange)
}
/// ditto
-R1 find(alias pred = "a == b", R1, R2)(R1 haystack, scope R2 needle)
+R1 find(alias pred = "a == b", R1, R2)(R1 haystack, scope R2 needle) @safe
if (isForwardRange!R1 && isForwardRange!R2
&& is(typeof(binaryFun!pred(haystack.front, needle.front)) : bool))
{
@@ -2408,7 +2408,7 @@ if (Ranges.length > 1 && is(typeof(startsWith!pred(haystack, needles))))
* such position exists, returns `haystack` advanced to termination).
*/
RandomAccessRange find(RandomAccessRange, alias pred, InputRange)(
- RandomAccessRange haystack, scope BoyerMooreFinder!(pred, InputRange) needle)
+ RandomAccessRange haystack, scope BoyerMooreFinder!(pred, InputRange) needle) @safe
{
return needle.beFound(haystack);
}
diff --git a/std/range/package.d b/std/range/package.d
index b179f98..e1609cc 100644
--- a/std/range/package.d
+++ b/std/range/package.d
@@ -9715,7 +9715,7 @@ if (isInputRange!Range)
{
return predFun(rhs, lhs);
}
- private Range _input;
+ /*private*/ Range _input;
// Undocummented because a clearer way to invoke is by calling
// assumeSorted.
make -f posix.mak std/algorithm/searching.test (aa[std.algorithm.searching]=-dip1000 in #6278)
results in:
...
T=`mktemp -d /tmp/.dmd-run-test.XXXXXX` && \
( \
../dmd/generated/linux/release/64/dmd -od$T -conf= -I../druntime/import -w -de -dip25 -m64 -fPIC -transition=complex -O -release -dip1000 -main -unittest generated/linux/release/64/libphobos2.a -defaultlib= -debuglib= -L-ldl -cov -run std/algorithm/searching.d ; \
RET=$? ; rm -rf $T ; exit $RET \
)
std/algorithm/searching.d(1969): Error: scope variable needle assigned to non-scope parameter r2 calling std.algorithm.comparison.mismatch!("a == b", SortedRange!(int[], "a < b"), SortedRange!(int[], "a < b")).mismatch
std/algorithm/searching.d(2101): Error: template instance `std.algorithm.searching.find!("a == b", SortedRange!(int[], "a < b"), SortedRange!(int[], "a < b"))` error instantiating
std/algorithm/searching.d(2413): Error: scope variable needle assigned to non-scope parameter this calling std.algorithm.searching.BoyerMooreFinder!(binaryFun, string).BoyerMooreFinder.beFound
std/algorithm/searching.d(2424): Error: template instance `std.algorithm.searching.find!(string, binaryFun, string)` error instantiating
std/algorithm/searching.d(2413): Error: scope variable needle assigned to non-scope parameter this calling std.algorithm.searching.BoyerMooreFinder!(binaryFun, int[]).BoyerMooreFinder.beFound
std/algorithm/searching.d(2436): Error: template instance `std.algorithm.searching.find!(int[], binaryFun, int[])` error instantiating
posix.mak:571: die Regel für Ziel „std/algorithm/searching.test“ scheiterte (the rule for target ... failed)
Thus, BoyerMooreFinder.beFound doesn't get its this parameter inferred scope.
And if You extend changes above to
Range beFound(Range haystack) scope @safe
the same is true (not inferred) for ptrdiff_t occurrence(ElementType!(Range) c):
std/algorithm/searching.d(379): Error: scope variable this assigned to non-scope parameter this calling std.algorithm.searching.BoyerMooreFinder!(binaryFun, string).BoyerMooreFinder.occurrence
std/algorithm/searching.d(1969): Error: scope variable needle assigned to non-scope parameter r2 calling std.algorithm.comparison.mismatch!("a == b", SortedRange!(int[], "a < b"), SortedRange!(int[], "a < b")).mismatch
std/algorithm/searching.d(2101): Error: template instance `std.algorithm.searching.find!("a == b", SortedRange!(int[], "a < b"), SortedRange!(int[], "a < b"))` error instantiating
std/algorithm/searching.d(379): Error: scope variable this assigned to non-scope parameter this calling std.algorithm.searching.BoyerMooreFinder!(binaryFun, int[]).BoyerMooreFinder.occurrence
std/algorithm/searching.d(2436): Error: template instance `std.algorithm.searching.find!(int[], binaryFun, int[])` error instantiating
posix.mak:571: die Regel für Ziel „std/algorithm/searching.test“ scheiterte (the rule for target ... failed)
I'm about to push an update for this PR
There was a problem hiding this comment.
Just a spontaneous idea: couldn't inout be modified to also infer scope?
There was a problem hiding this comment.
No, inout converts implicitly to const, it could be legally escaped. It also could be new data from the heap if returned.
|
Since I opened this PR 12 days ago, the errors changed to these: The additional ones can be removed again by removing protection attribute private from: |
|
Since this is congruent with the D vision, I added the Vision label and this is a priority issue. https://wiki.dlang.org/Vision/2018H1 |
|
If it still compiles successfully and passes the test suite after adding |
Eliminates an error from std.algorithm.searching (the remaining errors are from std.algorithm.comparison and std.uni):
make -f posix.mak std/algorithm/searching.test [-dip1000 for std.algorithm.searching.d (the posix.mak I'm using is master+ https://github.com//pull/6195] + -dip1000 for searching.d; master at Latest commit 4820a49)]:
PR applied and function
R1 find(alias pred = "a == b", R1, R2)(R1 haystack, scope R2 needle)
temporarily attributed @safe, results in: