fix issue 21592 - two stack traces if high surrogate is printed #7801
Conversation
|
Thanks for your pull request and interest in making D better, @aG0aep6G! 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 run digger -- build "master + phobos#7801" |
|
An alternative fix would be to reset import std.stdio;
import std.utf : UTFException;
void main()
{
auto w = stdout.lockingTextWriter();
w.put("\U00010400"w[0]);
try w.put('\n');
catch (UTFException e) w.put("\U00010400"w[1]);
} |
|
This doesn't work like I thought it would. I'll have to take another look. |
Fixed. I'm not happy with throwing on incomplete UTF-16 while silently ignoring incomplete UTF-8 sequences. But sorting that out is beyond the scope of this PR. Good to go from my side. |
|
Perhaps I'm missing something obvious, but it seems that this shorter implementation doesn't have this bug (or at least the unittest does not trigger it): void write(S...)(S args)
{
import std.format : formattedWrite;
import std.stdio : stdout;
import std.typecons : tuple;
auto w = lockingTextWriter();
formattedWrite(w, "%(%s%|%)", tuple(args));
}Full example: import std.stdio : File;
@safe unittest // https://issues.dlang.org/show_bug.cgi?id=21592
{
import std.exception : collectException;
import std.utf : UTFException;
static import std.file;
auto deleteme = "deleteme.txt";
scope(exit) std.file.remove(deleteme);
auto f = File(deleteme, "w");
auto e = collectException!UTFException(f.myWriteln(wchar(0xD801)));
assert(e.next is null);
}
void myWriteln(S...)(ref File f, S args)
{
myWrite(f, args, '\n');
}
void myWrite(S...)(File f, S args)
{
import std.format : formattedWrite;
import std.stdio : stdout;
import std.typecons : tuple;
auto w = f.lockingTextWriter();
formattedWrite(w, "%(%s%|%)", tuple(args));
}https://run.dlang.io/is/99SIV9 If the Edit: One disadvantage is that the stack trace is longer: https://run.dlang.io/is/QHvKWh vs. https://run.dlang.io/is/OBTWQE But this can be handled by catching and re-throwing the exception, similar to what is done in this PR. |
I didn't touch string handling yet. I will have to dive deeper into unicode before doing so. So I fear I cannot say anything qualified here. The only thing I know: There are some known bugs in the implementation of string handling in |
Note that the exception message is different. Your void main()
{
import std.stdio: stdout;
stdout.write("\U00010400"w[0], "\U00010400"w[1], "\n"); /* prints "𐐀" */
stdout.myWrite("\U00010400"w[0], "\U00010400"w[1], "\n"); /* throws */
} |
IMO, this is not worth supporting, unless it was already guaranteed by the documentation and unit tests. |
That's alright, but I just want to fix a bug. I'd rather not rewrite the whole function, changing its behavior in subtle ways. |
|
Okay, fair enough. I restarted the druntime job on BuildKite as it was failing due to running out of memory. I also restarted FreeBSD_32 job on the auto-tester. |
|
This seems to consistently fail druntime on buildkite. The error message: |
I don't see how the changes here could cause undefined references. |
|
Maybe this is due to the split of Anyway, it would be nice to know why this fails. Might be something like "private members of std/format/package.d are treated differently to private members of std/format.d" or something... |
|
Yeah, rebasing might fix it. |
|
Strange things are happening here: I was able to reproduce the error locally by running PS: Even with a clean newly checked out master of all three parts I get this error... PPS: Rewinding all three to 13th of Februar, 17:06 brought up similar errors. So I conclude, that this is neither related to this PR nor to the split of But I might be wrong and using the tests in |
|
rebased |
No description provided.