EricWF added a comment.
Looking good. I would like to see some tests for the methods we have specificly
defined to have behavior in -fno-exceptions mode.
================
Comment at: src/cxa_noexception.cpp:27
@@ +26,3 @@
+
+_LIBCXXABI_FUNC_VIS void
+__cxa_increment_exception_refcount(void *thrown_object) throw() {
----------------
I think either all of the functions in this file should have
`_LIBCXXABI_FUNC_VIS` or none of them should. My preference is none since the
declarations should have them.
================
Comment at: src/cxa_noexception.cpp:29
@@ +28,3 @@
+__cxa_increment_exception_refcount(void *thrown_object) throw() {
+ if (thrown_object != NULL)
+ std::terminate();
----------------
nit: nullptr
================
Comment at: src/cxa_noexception.cpp:45
@@ +44,3 @@
+void
+__cxa_rethrow_primary_exception(void*) { return; }
+
----------------
I think we should terminate if `void*` is non-null.
================
Comment at: test/backtrace_test.pass.cpp:19
@@ -19,2 +19,2 @@
}
----------------
Does this test have any value in -fno-exceptions mode?
================
Comment at: test/libcxxabi/test/config.py:46
@@ +45,3 @@
+ if self.get_lit_bool('enable_exceptions', True):
+ self.cxx.compile_flags += ['-funwind-tables',
'-DLIBCXXABI_ENABLE_EXCEPTIONS']
+ else:
----------------
Nit: I would prefer `LIBCXXABI_HAS_NO_EXCEPTIONS` since the default should be
on. The fact that a macro is missing shouldn't disable half the tests.
================
Comment at: test/test_vector1.pass.cpp:8
@@ -7,3 +7,3 @@
//
//===----------------------------------------------------------------------===//
----------------
Don't you want these tests to run? They aren't part of the exception API AFAIK.
================
Comment at: test/uncaught_exceptions.pass.cpp:10
@@ -9,1 +9,3 @@
+// UNSUPPORTED: libcxxabi-no-exceptions
+
----------------
We provided a different implementation so we should test it, not disable the
test.
http://reviews.llvm.org/D20677
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
