<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">On Nov 12, 2017, at 12:37, Reimar Döffinger <<a href="mailto:Reimar.Doeffinger@gmx.de" class="">Reimar.Doeffinger@gmx.de</a>> wrote:<br class=""><div><blockquote type="cite" class=""><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">On Thu, Nov 09, 2017 at 05:37:32PM -0800, Volodymyr Sapsai wrote:</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">On Nov 9, 2017, at 12:13, Reimar Döffinger <<a href="mailto:Reimar.Doeffinger@gmx.de" class="">Reimar.Doeffinger@gmx.de</a>> wrote:<br class=""><blockquote type="cite" class=""><br class="">Hello!<br class=""><br class="">On Wed, Nov 08, 2017 at 12:36:00PM -0800, Volodymyr Sapsai wrote:<br class=""><blockquote type="cite" class="">Thanks for the patch, Reimar. Can you please add tests to ensure this functionality doesn’t regress? As null character is required by the standard (27.7.2.3p21), a good starting point seems to be<br class="">test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp<br class="">test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp<br class=""></blockquote><br class="">New patch attached, though I went the lazy way of just adding one case.<br class="">Not sure that's "good enough" - in principle I think it should be.<br class=""></blockquote>I think it makes sense to cover wchar_t as well. And I think it would be useful to test the case with specified delimiter too. Most likely implementation should be reused for the case with explicit delimiter and default new line delimiter. But I prefer not to rely on this assumption and test explicitly.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Well, it was rather trivial to do anyway, so done.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote>Thanks for adding more tests.</div><div><br class=""><blockquote type="cite" class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class="">More tricky would be to add a test for the _LIBCPP_NO_EXCEPTIONS case,<br class="">is there any code testing that at all?<br class=""></blockquote>According to tests, exceptions are tested in various ways. Let me find how to configure the build to run tests in these modes.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">There aren't many references to that mode, and I am not sure</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">running in a separate mode would make for a good test anyway.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Maybe not how it should done, but for now I added a separate file</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">that just defines _LIBCPP_NO_EXCEPTIONS before the includes.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span id="cid:9574EBD2-5A02-4D6B-A2B4-FBCC0E015959@corp.apple.com"><0001-Ensure-std-istream-getline-always-0-terminates-strin.patch></span></div></blockquote></div><br class=""><div class="">libc++ can be built with exceptions enabled or disabled (see <a href="http://libcxx.llvm.org/docs/BuildingLibcxx.html#cmdoption-arg-libcxx-enable-exceptions" class="">LIBCXX_ENABLE_EXCEPTIONS</a>) and we need to support both configurations. The problem with your _LIBCPP_NO_EXCEPTIONS approach is that it won't work when exceptions are disabled. Various exception-related tests are usually guarded with TEST_HAS_NO_EXCEPTIONS macro defined in “test_macros.h”. You can check other tests for examples of TEST_HAS_NO_EXCEPTIONS usage, I had in mind something like</div><div class=""><br class=""></div><div class="">#ifndef TEST_HAS_NO_EXCEPTIONS<br class=""> {<br class=""> testbuf<char> sb(" ");<br class=""> std::istream is(&sb);<br class=""> char s[5] = "test";<br class=""> is.exceptions(std::istream::eofbit | std::istream::badbit);<br class=""> try<br class=""> {<br class=""> is.getline(s, 5);<br class=""> assert(false);<br class=""> }<br class=""> catch (std::ios_base::failure&)<br class=""> {<br class=""> }<br class=""> assert( is.eof());<br class=""> assert( is.fail());<br class=""> assert(std::string(s) == " ");<br class=""> assert(is.gcount() == 1);<br class=""> }<br class="">#endif</div><div class=""><br class=""></div><div class="">Non-empty `sb` allows to test null-termination at correct place in array; initializing `s` with “test” avoids spurious correct values in memory; try-catch block and badbit are to make it more obvious we are testing code path throwing an exception.</div></body></html>