[PATCH] D21706: [libcxx] refactor for throw

Weiming Zhao via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 1 13:38:23 PDT 2016


weimingz added inline comments.

================
Comment at: include/exception:262
@@ -261,3 +261,3 @@
 _LIBCPP_INLINE_VISIBILITY
-inline void __libcpp_throw(_Exception const& __e) {
+inline __attribute__((noreturn)) void __libcpp_throw(_Exception const& __e) {
 #ifndef _LIBCPP_NO_EXCEPTIONS
----------------
mclow.lists wrote:
> I thought that we were going to add a macro for the "no return if no exceptions"
something like this?

#ifndef __LIBCPP_NO_EXCEPTIONS
  #define NORETURN_EXP __attribute__((noreturn))
#else
  #define NORETURN_EXP
#endif

inline NORETURN_EXP void __libcpp_throw(_Exception const& __e) {
  ...
}

================
Comment at: src/future.cpp:96
@@ -96,4 +95,3 @@
     if (__has_value())
-        throw future_error(make_error_code(future_errc::promise_already_satisfied));
-#endif
+        __libcpp_throw(future_error(make_error_code(future_errc::promise_already_satisfied)));
     __state_ |= __constructed | ready;
----------------
mclow.lists wrote:
> Is the indentation right here? (or is phab lying to me?)
It looks ok here. The first underscore  is on the same column as "__has_value" (the line above). 4 space indention

================
Comment at: src/locale.cpp:527
@@ -528,3 +526,3 @@
 #else  // _LIBCPP_NO_EXCEPTIONS
     : __locale_(new __imp(*other.__locale_, name, c))
 #endif
----------------
mclow.lists wrote:
> Did you miss one here?
The reason I skipeed this and two or three other similar cases is the constructor expects the function __libcpp_throw() to return a pointer. 


http://reviews.llvm.org/D21706





More information about the cfe-commits mailing list