From 62b9c0c128cab8c4f51cab1546c201555844f0df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 5 Oct 2022 14:18:57 +0200 Subject: [PATCH 01/10] Generalize dirEntries to take a DirEntry predicate as parameter --- std/file.d | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/std/file.d b/std/file.d index 8d1b431215c..88755dc7b0c 100644 --- a/std/file.d +++ b/std/file.d @@ -4617,7 +4617,8 @@ enum SpanMode ["animals", "plants"])); } -private struct DirIteratorImpl +private struct DirIteratorImpl(alias pred = (const scope ref DirEntry entry) => true) +// TODO: if (is(typeof(pred(DirEntry.init)) : bool)) { @safe: SpanMode _mode; @@ -4721,6 +4722,8 @@ private struct DirIteratorImpl bool mayStepIn() { + if (pred(_cur)) + return false; return _followSymlink ? _cur.isDir : _cur.isDir && !_cur.isSymlink; } } @@ -4864,16 +4867,17 @@ private struct DirIteratorImpl } } +struct _DirIterator(bool useDIP1000, alias pred = (const scope ref DirEntry entry) => true) +// TODO: if (is(typeof(pred(DirEntry.init)) : bool)) // Must be a template, because the destructor is unsafe or safe depending on // whether `-preview=dip1000` is in use. Otherwise, linking errors would // result. -struct _DirIterator(bool useDIP1000) { static assert(useDIP1000 == dip1000Enabled, "Please don't override useDIP1000 to disagree with compiler switch."); private: - SafeRefCounted!(DirIteratorImpl, RefCountedAutoInitialize.no) impl; + SafeRefCounted!(DirIteratorImpl!(pred), RefCountedAutoInitialize.no) impl; this(string pathname, SpanMode mode, bool followSymlink) @trusted { @@ -4975,10 +4979,11 @@ foreach (d; dFiles) // For some reason, doing the same alias-to-a-template trick as with DirIterator // does not work here. -auto dirEntries(bool useDIP1000 = dip1000Enabled) +auto dirEntries(bool useDIP1000 = dip1000Enabled, alias pred = (const scope ref DirEntry entry) => true) (string path, SpanMode mode, bool followSymlink = true) +// TODO: if (is(typeof(pred(DirEntry.init)) : bool)) { - return _DirIterator!useDIP1000(path, mode, followSymlink); + return _DirIterator!(useDIP1000, pred)(path, mode, followSymlink); } /// Duplicate functionality of D1's `std.file.listdir()`: @@ -5087,7 +5092,7 @@ auto dirEntries(bool useDIP1000 = dip1000Enabled) import std.path : globMatch, baseName; bool f(DirEntry de) { return globMatch(baseName(de.name), pattern); } - return filter!f(_DirIterator!useDIP1000(path, mode, followSymlink)); + return filter!f(_DirIterator!(useDIP1000)(path, mode, followSymlink)); } @safe unittest From 221176b847f5a886ce699008a93c077f614dff5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 5 Oct 2022 22:52:58 +0200 Subject: [PATCH 02/10] Swap template parameters pred and useDIP1000 --- std/file.d | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/std/file.d b/std/file.d index 88755dc7b0c..aad97899508 100644 --- a/std/file.d +++ b/std/file.d @@ -4867,7 +4867,7 @@ private struct DirIteratorImpl(alias pred = (const scope ref DirEntry entry) => } } -struct _DirIterator(bool useDIP1000, alias pred = (const scope ref DirEntry entry) => true) +struct _DirIterator(alias pred = (const scope ref DirEntry entry) => true, bool useDIP1000) // TODO: if (is(typeof(pred(DirEntry.init)) : bool)) // Must be a template, because the destructor is unsafe or safe depending on // whether `-preview=dip1000` is in use. Otherwise, linking errors would @@ -4891,7 +4891,7 @@ public: // This has the client code to automatically use and link to the correct // template instance -alias DirIterator = _DirIterator!dip1000Enabled; +alias DirIterator = _DirIterator!((const scope ref DirEntry entry) => true, dip1000Enabled); /++ Returns an $(REF_ALTTEXT input range, isInputRange, std,range,primitives) @@ -4979,11 +4979,11 @@ foreach (d; dFiles) // For some reason, doing the same alias-to-a-template trick as with DirIterator // does not work here. -auto dirEntries(bool useDIP1000 = dip1000Enabled, alias pred = (const scope ref DirEntry entry) => true) +auto dirEntries(alias pred = (const scope ref DirEntry entry) => true, bool useDIP1000 = dip1000Enabled) (string path, SpanMode mode, bool followSymlink = true) // TODO: if (is(typeof(pred(DirEntry.init)) : bool)) { - return _DirIterator!(useDIP1000, pred)(path, mode, followSymlink); + return _DirIterator!(pred, useDIP1000)(path, mode, followSymlink); } /// Duplicate functionality of D1's `std.file.listdir()`: @@ -5084,7 +5084,8 @@ auto dirEntries(bool useDIP1000 = dip1000Enabled, alias pred = (const scope ref } /// Ditto -auto dirEntries(bool useDIP1000 = dip1000Enabled) +auto dirEntries(alias pred = (const scope ref DirEntry entry) => true, + bool useDIP1000 = dip1000Enabled) (string path, string pattern, SpanMode mode, bool followSymlink = true) { @@ -5092,7 +5093,7 @@ auto dirEntries(bool useDIP1000 = dip1000Enabled) import std.path : globMatch, baseName; bool f(DirEntry de) { return globMatch(baseName(de.name), pattern); } - return filter!f(_DirIterator!(useDIP1000)(path, mode, followSymlink)); + return filter!f(_DirIterator!(pred, useDIP1000)(path, mode, followSymlink)); } @safe unittest From 4624f2e10a8e925960f633561ac1b9719b6c7391 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 5 Oct 2022 23:03:10 +0200 Subject: [PATCH 03/10] Remove const ref from pred parameter and activate template restrictions --- std/file.d | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/std/file.d b/std/file.d index aad97899508..aa19ca8c3e1 100644 --- a/std/file.d +++ b/std/file.d @@ -4617,8 +4617,8 @@ enum SpanMode ["animals", "plants"])); } -private struct DirIteratorImpl(alias pred = (const scope ref DirEntry entry) => true) -// TODO: if (is(typeof(pred(DirEntry.init)) : bool)) +private struct DirIteratorImpl(alias pred = (scope DirEntry entry) => true) +if (is(typeof(pred(DirEntry.init)) : bool)) { @safe: SpanMode _mode; @@ -4867,8 +4867,8 @@ private struct DirIteratorImpl(alias pred = (const scope ref DirEntry entry) => } } -struct _DirIterator(alias pred = (const scope ref DirEntry entry) => true, bool useDIP1000) -// TODO: if (is(typeof(pred(DirEntry.init)) : bool)) +struct _DirIterator(alias pred = (scope DirEntry entry) => true, bool useDIP1000) +if (is(typeof(pred(DirEntry.init)) : bool)) // Must be a template, because the destructor is unsafe or safe depending on // whether `-preview=dip1000` is in use. Otherwise, linking errors would // result. @@ -4891,7 +4891,7 @@ public: // This has the client code to automatically use and link to the correct // template instance -alias DirIterator = _DirIterator!((const scope ref DirEntry entry) => true, dip1000Enabled); +alias DirIterator = _DirIterator!((scope DirEntry entry) => true, dip1000Enabled); /++ Returns an $(REF_ALTTEXT input range, isInputRange, std,range,primitives) @@ -4979,9 +4979,9 @@ foreach (d; dFiles) // For some reason, doing the same alias-to-a-template trick as with DirIterator // does not work here. -auto dirEntries(alias pred = (const scope ref DirEntry entry) => true, bool useDIP1000 = dip1000Enabled) +auto dirEntries(alias pred = (scope DirEntry entry) => true, bool useDIP1000 = dip1000Enabled) (string path, SpanMode mode, bool followSymlink = true) -// TODO: if (is(typeof(pred(DirEntry.init)) : bool)) +if (is(typeof(pred(DirEntry.init)) : bool)) { return _DirIterator!(pred, useDIP1000)(path, mode, followSymlink); } @@ -5084,7 +5084,7 @@ auto dirEntries(alias pred = (const scope ref DirEntry entry) => true, bool useD } /// Ditto -auto dirEntries(alias pred = (const scope ref DirEntry entry) => true, +auto dirEntries(alias pred = (scope DirEntry entry) => true, bool useDIP1000 = dip1000Enabled) (string path, string pattern, SpanMode mode, bool followSymlink = true) From 00ea32db73f22943840007d848970e64f72ea668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 6 Oct 2022 08:55:48 +0200 Subject: [PATCH 04/10] Pass entry by ref in pred in use __traits(compiles) instead of is() --- std/file.d | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/std/file.d b/std/file.d index aa19ca8c3e1..0890f40e621 100644 --- a/std/file.d +++ b/std/file.d @@ -4617,8 +4617,8 @@ enum SpanMode ["animals", "plants"])); } -private struct DirIteratorImpl(alias pred = (scope DirEntry entry) => true) -if (is(typeof(pred(DirEntry.init)) : bool)) +private struct DirIteratorImpl(alias pred = (scope ref DirEntry entry) => true) +if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) { @safe: SpanMode _mode; @@ -4867,8 +4867,8 @@ if (is(typeof(pred(DirEntry.init)) : bool)) } } -struct _DirIterator(alias pred = (scope DirEntry entry) => true, bool useDIP1000) -if (is(typeof(pred(DirEntry.init)) : bool)) +struct _DirIterator(alias pred = (scope ref DirEntry entry) => true, bool useDIP1000) +if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) // Must be a template, because the destructor is unsafe or safe depending on // whether `-preview=dip1000` is in use. Otherwise, linking errors would // result. @@ -4891,7 +4891,7 @@ public: // This has the client code to automatically use and link to the correct // template instance -alias DirIterator = _DirIterator!((scope DirEntry entry) => true, dip1000Enabled); +alias DirIterator = _DirIterator!((scope ref DirEntry entry) => true, dip1000Enabled); /++ Returns an $(REF_ALTTEXT input range, isInputRange, std,range,primitives) @@ -4979,9 +4979,9 @@ foreach (d; dFiles) // For some reason, doing the same alias-to-a-template trick as with DirIterator // does not work here. -auto dirEntries(alias pred = (scope DirEntry entry) => true, bool useDIP1000 = dip1000Enabled) +auto dirEntries(alias pred = (scope ref DirEntry entry) => true, bool useDIP1000 = dip1000Enabled) (string path, SpanMode mode, bool followSymlink = true) -if (is(typeof(pred(DirEntry.init)) : bool)) +if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) { return _DirIterator!(pred, useDIP1000)(path, mode, followSymlink); } @@ -5084,10 +5084,10 @@ if (is(typeof(pred(DirEntry.init)) : bool)) } /// Ditto -auto dirEntries(alias pred = (scope DirEntry entry) => true, - bool useDIP1000 = dip1000Enabled) +auto dirEntries(alias pred = (scope ref DirEntry entry) => true, bool useDIP1000 = dip1000Enabled) (string path, string pattern, SpanMode mode, bool followSymlink = true) +if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) { import std.algorithm.iteration : filter; import std.path : globMatch, baseName; From 5741571f6390ca4da41eb640736f96b304e4c25b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 6 Oct 2022 09:00:27 +0200 Subject: [PATCH 05/10] Correct logic in mayStepIn --- std/file.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/file.d b/std/file.d index 0890f40e621..5f34a3f9172 100644 --- a/std/file.d +++ b/std/file.d @@ -4722,7 +4722,7 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) bool mayStepIn() { - if (pred(_cur)) + if (!pred(_cur)) return false; return _followSymlink ? _cur.isDir : _cur.isDir && !_cur.isSymlink; } From eea0b72a334647b8919feabe5260a8d484e490c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 6 Oct 2022 09:12:06 +0200 Subject: [PATCH 06/10] Add failing test --- std/file.d | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/std/file.d b/std/file.d index 5f34a3f9172..7953976c291 100644 --- a/std/file.d +++ b/std/file.d @@ -5198,6 +5198,12 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) sort(result); assert(equal(files, result)); + + import std.algorithm : endsWith; + auto result2 = dirEntries!((scope ref DirEntry entry) => entry.name.endsWith("Hello World"))(dir, SpanMode.shallow).map!((return a) => a.name.normalize()).array(); + import std.stdio; + writeln(result2); + assert(result2.length == 1); } // https://issues.dlang.org/show_bug.cgi?id=21250 From 8ba61f449a929f2a5831a67e52e74724adb1c2ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 12 Oct 2022 11:30:28 +0200 Subject: [PATCH 07/10] Use pred in posix version of mayStepIn --- std/file.d | 2 ++ 1 file changed, 2 insertions(+) diff --git a/std/file.d b/std/file.d index 7953976c291..291c23361d5 100644 --- a/std/file.d +++ b/std/file.d @@ -4783,6 +4783,8 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) bool mayStepIn() { + if (!pred(_cur)) + return false; return _followSymlink ? _cur.isDir : attrIsDir(_cur.linkAttributes); } } From d68117965ea816f347dac9989128c51a05d9849b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 12 Oct 2022 11:37:30 +0200 Subject: [PATCH 08/10] Try to use pred in DirIteratorImpl --- std/file.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/file.d b/std/file.d index 291c23361d5..552ad20d4f2 100644 --- a/std/file.d +++ b/std/file.d @@ -4704,7 +4704,7 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) return false; } _cur = DirEntry(_stack[$-1].dirpath, findinfo); - return true; + return pred(_cur); } void popDirStack() @trusted @@ -4760,7 +4760,7 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) core.stdc.string.strcmp(&fdata.d_name[0], "..")) { _cur = DirEntry(_stack[$-1].dirpath, fdata); - return true; + return pred(_cur); } } From 77822bff7bff4b3b0a369fb8423f7c6d468614af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 12 Oct 2022 11:39:43 +0200 Subject: [PATCH 09/10] Use final switch in DirIteratorImpl.popFront --- std/file.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/std/file.d b/std/file.d index 552ad20d4f2..d0227004cb9 100644 --- a/std/file.d +++ b/std/file.d @@ -4830,7 +4830,7 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) void popFront() { - switch (_mode) + final switch (_mode) { case SpanMode.depth: if (next()) @@ -4858,7 +4858,7 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) else while (!empty && !next()){} break; - default: + case SpanMode.shallow: next(); } } From 308a9dc377987b02e3dc7394da6831adf1c8c64f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 12 Oct 2022 11:43:02 +0200 Subject: [PATCH 10/10] Make test cover all span modes --- std/file.d | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/std/file.d b/std/file.d index d0227004cb9..a02a7beea5f 100644 --- a/std/file.d +++ b/std/file.d @@ -5201,11 +5201,13 @@ if (__traits(compiles, { DirEntry entry; bool _ = pred(entry); })) assert(equal(files, result)); - import std.algorithm : endsWith; - auto result2 = dirEntries!((scope ref DirEntry entry) => entry.name.endsWith("Hello World"))(dir, SpanMode.shallow).map!((return a) => a.name.normalize()).array(); - import std.stdio; - writeln(result2); - assert(result2.length == 1); + foreach (const spanMode; [EnumMembers!(SpanMode)]) { + import std.algorithm : endsWith; + auto result2 = dirEntries!((scope ref DirEntry entry) => entry.name.endsWith("Hello World"))(dir, spanMode).map!((return a) => a.name.normalize()).array(); + import std.stdio; + writeln(result2); + assert(result2.length == 1); + } } // https://issues.dlang.org/show_bug.cgi?id=21250