On Nov 19, 2017, at 08:07, Reimar Döffinger <reimar.doeffin...@gmx.de> wrote: > > On Wed, Nov 15, 2017 at 11:35:56AM -0800, Volodymyr Sapsai wrote: >> On Nov 12, 2017, at 12:37, Reimar Döffinger <reimar.doeffin...@gmx.de> wrote: >> 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); > > That doesn't make sense to me, the whole point is to test the > code-path in getline that swallows the exception. > So if you meant for that assert to be there, you're > not trying to test what I am trying to test. > Note that the intent is not to ensure 0-termination > when getline actually exits via an exception, I doubt > that would be safe to do really (except maybe at the start > of the loop before the code that might trigger an exception, > but that would cost performance) and anyway code > handling exceptions can be expected to not make assumptions > that the result is in a "safe" state. > The problem is when the code just swallows the exception > (when exceptions are disabled, presumably for interoperability > with code compiled with exceptions enabled? There is no documentation > why getline catches exceptions when they are disabled), > then I think the only reasonable choice is to 0-terminate, > even if it is risky. > Looking at the other tests it seems that generally functionality > simply isn't tested at all when exceptions are disabled > (e.g. stoul.pass.cpp), which seems unfortunate. > That level of testing is easy to achieve here as well, > just leave out the test file I added. > If you want it though, I updated it to not fail (though > not actually test anything either) if exceptions are > disabled, and added a comment on what a huge hack it is > (as the code-path for disabled exceptions is tested if and > only if exceptions are actually enabled). > <0001-Ensure-std-istream-getline-always-0-terminates-strin.patch>
I got confused here. When you mention "code path for disabled exceptions”, what exactly code do you have in mind? Is that > - if (__n > 0) > - *__s = char_type(); > if (__gc_ == 0) > __err |= ios_base::failbit; > this->setstate(__err); > } > + if (__n > 0) > + *__s = char_type(); or > catch (...) > { > + if (__n > 0) > + *__s = char_type(); > this->__set_badbit_and_consider_rethrow(); > } or maybe something else? _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits