Skip to content

Conversation

rbanderton
Copy link
Contributor

https://issues.dlang.org/show_bug.cgi?id=6644
More discussion here: http://stackoverflow.com/questions/19101305/how-can-i-do-i-o-in-safe-functions-in-d

Changes I have made to get write and family callable from @safe code:

  • Made LockingTextWriter and factory function @trusted - I didn't see anything inherently unsafe about the way it manages the lock. I'm certainly open to correction, though.
  • Wrapped the stdout global variable's usage in a private @trusted property that is used by write and friends. If this is not done they always infer to @system due to accessing a global variable. I'm not 100% sure this is safe to to, but with the property being private all external use of the global will still be @system.

This is much like #2230 except it does not use free functions - after more testing I found they worked fine as member functions. This is a separate pull request because I'm terrible at git :/

@@ -2208,7 +2208,7 @@ $(D Range) that locks the file and allows fast writing to it.

See $(LREF byChunk) for an example.
*/
auto lockingTextWriter()
auto lockingTextWriter() @trusted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of blunt, see #2225

@quickfur
Copy link
Member

ping
Any updates / responses to the comments?

@mihails-strasuns
Copy link

This needs to be updated on top of #2225 - @trusted must be applied only to minimal necessary parts of code, not whole functions.

@rbanderton
Copy link
Contributor Author

Will update this to work with #2225.

@rbanderton
Copy link
Contributor Author

Updated to be compatible with #2225, but it looks like more changes from #2321 are needed for this to work. I'll add the same [DO NOT MERGE] to the PR title since this relies on that PR.

@rbanderton rbanderton changed the title Fix Issue 6644 - std.stdio write/writef(ln) are not @trusted [DO NOT MERGE] Fix Issue 6644 - std.stdio write/writef(ln) are not @trusted Jul 13, 2014
@mihails-strasuns
Copy link

#2321 was merged, please update/rebase this, currently fails the auto-tester

@rbanderton rbanderton changed the title [DO NOT MERGE] Fix Issue 6644 - std.stdio write/writef(ln) are not @trusted Fix Issue 6644 - std.stdio write/writef(ln) are not @trusted Jul 29, 2014
@rbanderton
Copy link
Contributor Author

Updated to be compatible with #2321. Tests pass locally and will fix any issues that come up in the auto-tester.

@mihails-strasuns
Copy link

Thanks. LGTM from me, need one more reviewer

}
}

@trusted void trustedTests()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System is the same as trusted, as far as the allowed operations are concerned, so just get rid of this section.

@dnadlinger
Copy link
Contributor

@rbanderton: Thanks a lot, looks good to me now! One last request: Could you please squash the commits together into one?

…rusted private function so writeln (and family) free functions don't always infer @System.
@rbanderton
Copy link
Contributor Author

Of course. Rebased to master/squashed commits.

@quickfur
Copy link
Member

LGTM. This will make std.stdio (more) usable in @safe code. I like it.

@dnadlinger
Copy link
Contributor

Auto-merge toggled on

dnadlinger added a commit that referenced this pull request Jul 30, 2014
Fix Issue 6644 - std.stdio write/writef(ln) are not @trusted
@dnadlinger dnadlinger merged commit 72aa8a9 into dlang:master Jul 30, 2014
@rbanderton rbanderton deleted the fix-writeln-inference branch July 30, 2014 20:20
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

Successfully merging this pull request may close these issues.

5 participants