On Nov 12, 2017, at 12:37, Reimar Döffinger <reimar.doeffin...@gmx.de> wrote:
>
> On Thu, Nov 09, 2017 at 05:37:32PM -0800, Volodymyr Sapsai wrote:
>> On Nov 9, 2017, at 12:13, Reimar Döffinger <reimar.doeffin...@gmx.de> wrote:
>>>
>>> Hello!
>>>
>>> On Wed, Nov 08, 2017 at 12:36:00PM -0800, Volodymyr Sapsai wrote:
>>>> 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
>>>> test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size.pass.cpp
>>>> test/std/input.output/iostream.format/input.streams/istream.unformatted/getline_pointer_size_chart.pass.cpp
>>>
>>> New patch attached, though I went the lazy way of just adding one case.
>>> Not sure that's "good enough" - in principle I think it should be.
>> 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.
>
> Well, it was rather trivial to do anyway, so done.
Thanks for adding more tests.
>>> More tricky would be to add a test for the _LIBCPP_NO_EXCEPTIONS case,
>>> is there any code testing that at all?
>> According to tests, exceptions are tested in various ways. Let me find how
>> to configure the build to run tests in these modes.
>
> There aren't many references to that mode, and I am not sure
> running in a separate mode would make for a good test anyway.
> Maybe not how it should done, but for now I added a separate file
> that just defines _LIBCPP_NO_EXCEPTIONS before the includes.
> <0001-Ensure-std-istream-getline-always-0-terminates-strin.patch>
libc++ can be built with exceptions enabled or disabled (see
LIBCXX_ENABLE_EXCEPTIONS
<http://libcxx.llvm.org/docs/BuildingLibcxx.html#cmdoption-arg-libcxx-enable-exceptions>)
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
#ifndef TEST_HAS_NO_EXCEPTIONS
{
testbuf<char> sb(" ");
std::istream is(&sb);
char s[5] = "test";
is.exceptions(std::istream::eofbit | std::istream::badbit);
try
{
is.getline(s, 5);
assert(false);
}
catch (std::ios_base::failure&)
{
}
assert( is.eof());
assert( is.fail());
assert(std::string(s) == " ");
assert(is.gcount() == 1);
}
#endif
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._______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
