[libcxx-commits] [PATCH] D103947: [libcxx] Fix using the vcruntime ABI with _HAS_EXCEPTIONS=0 defined

[Petr Hosek via Phabricator via libcxx-commits]

phosek added inline comments.


================
Comment at: libcxx/include/exception:102
+#if defined(_LIBCPP_ABI_VCRUNTIME) && (!defined(_HAS_EXCEPTIONS) || _HAS_EXCEPTIONS == 1)
+// The Exception class was already included above, but we're explicit about this condition here for clarity
+
----------------
The name of the class isn't capitalized.


================
Comment at: libcxx/include/exception:112
+
+// Note: details for an ABI compatable implementation were taken from:
+// https://github.com/microsoft/STL/blob/afe0800cd6bf0d163bf678ad4e1feb3c130d1ec5/stl/inc/exception#L80
----------------
Typo.


================
Comment at: libcxx/include/exception:118-119
+
+    // this constructor is necessary to compile
+    // successfully header new for _HAS_EXCEPTIONS==0 scenario
+    explicit exception(const char* _Message = "unknown", int = 1) _NOEXCEPT {}
----------------
I'd make it a full sentence.


================
Comment at: libcxx/include/exception:122
+
+    exception(const exception& _Right) _NOEXCEPT {}
+
----------------
You can omit the argument name since it's unused.


================
Comment at: libcxx/include/exception:124
+
+    exception& operator=(const exception& _Right) _NOEXCEPT {
+        return *this;
----------------
You can omit the argument name since it's unused.


================
Comment at: libcxx/include/exception:134-135
+
+    _LIBCPP_NORETURN void _Raise() const {
+    }
+
----------------
Nit


================
Comment at: libcxx/include/exception:140
+
+protected:
+    const char* _Ptr; 
----------------
This is not necessary since you already have `protected:` above.


================
Comment at: libcxx/include/exception:141
+protected:
+    const char* _Ptr; 
+};
----------------
I wonder if we should annotate this with `[[maybe_unused]]` to avoid a compiler warning.


================
Comment at: libcxx/include/exception:151-152
+protected:
+    void _Doraise() const { 
+    }
+};
----------------
Nit


================
Comment at: libcxx/include/exception:155
+
+#else// !defined(_LIBCPP_ABI_VCRUNTIME)
+// On all other platforms, we define our own std::exception and std::bad_exception types
----------------
Nit


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103947/new/

https://reviews.llvm.org/D103947