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.

Sign up for GitHub

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 #10605 - Make File.byLine usable in @safe contexts #10622

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Fix #10605 - Make File.byLine usable in @safe contexts #10622

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 27 additions & 13 deletions std/stdio.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2169,13 +2169,13 @@ Allows to directly use range operations on lines of a file.
private struct ByLineImpl(Char, Terminator)
{
private:
import std.typecons : RefCounted, RefCountedAutoInitialize;
import std.typecons : borrow, RefCountedAutoInitialize, SafeRefCounted;

/* Ref-counting stops the source range's Impl
* from getting out of sync after the range is copied, e.g.
* when accessing range.front, then using std.range.take,
* then accessing range.front again. */
alias PImpl = RefCounted!(Impl, RefCountedAutoInitialize.no);
alias PImpl = SafeRefCounted!(Impl, RefCountedAutoInitialize.no);
PImpl impl;

static if (isScalarType!Terminator)
Expand All @@ -2190,19 +2190,24 @@ Allows to directly use range operations on lines of a file.
impl = PImpl(f, kt, terminator);
}

@property bool empty()
/* Verifiably `@safe` when built with -preview=DIP1000. */
@property bool empty() @trusted
{
return impl.refCountedPayload.empty;
// Using `ref` is actually necessary here.
return impl.borrow!((ref i) => i.empty);
}

@property Char[] front()
/* Verifiably `@safe` when built with -preview=DIP1000. */
@property Char[] front() @trusted
{
return impl.refCountedPayload.front;
// Using `ref` is likely optional here.
return impl.borrow!((ref i) => i.front);
}

void popFront()
/* Verifiably `@safe` when built with -preview=DIP1000. */
void popFront() @trusted
{
impl.refCountedPayload.popFront();
return impl.borrow!((ref i) => i.popFront());
}

private:
Expand All @@ -2216,6 +2221,7 @@ Allows to directly use range operations on lines of a file.
KeepTerminator keepTerminator;
bool haveLine;

@safe:
public:
this(File f, KeepTerminator kt, Terminator terminator)
{
Expand Down Expand Up @@ -2375,7 +2381,7 @@ void main()
return ByLineImpl!(Char, Terminator)(this, keepTerminator, terminator);
}

@system unittest
@safe unittest
{
static import std.file;
auto deleteme = testFilename();
Expand All @@ -2393,7 +2399,7 @@ void main()
}

// https://issues.dlang.org/show_bug.cgi?id=19980
@system unittest
@safe unittest
{
static import std.file;
auto deleteme = testFilename();
Expand Down Expand Up @@ -2541,12 +2547,11 @@ $(REF readText, std,file)
is(typeof(File("").byLineCopy!(char, char).front) == char[]));
}

@system unittest
@safe unittest
{
import std.algorithm.comparison : equal;
static import std.file;

scope(failure) printf("Failed test at line %d\n", __LINE__);
auto deleteme = testFilename();
std.file.write(deleteme, "");
scope(success) std.file.remove(deleteme);
Expand Down Expand Up @@ -2620,7 +2625,7 @@ $(REF readText, std,file)
test("sue\r", ["sue\r"], kt, '\r');
}

@system unittest
@safe unittest
{
import std.algorithm.comparison : equal;
import std.range : drop, take;
Expand Down Expand Up @@ -4765,6 +4770,15 @@ struct lines
foreach (line; myByLineCopy)
continue;
}

{
auto f = File(deleteMe, "r");
scope(exit) { f.close(); }

auto myByLine = f.byLine;
foreach (line; myByLine)
continue;
}
}

@system unittest
Expand Down
Loading