[PATCH] D38209: [Sema] Correct nothrow inherited by noexcept

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm not certain we have the semantics of `__declspec(nothrow)` exactly correct -- a simple test shows that `__attribute__((nothrow))`, `__declspec(nothrow)`, and `noexcept(true)` are subtly different.

When I run this with MSVC 2017, the terminate handler is not called and the application continues rather than terminates, but Clang calls the terminate handler.

  #include <exception>
  #include <iostream>
  
  __declspec(nothrow) void f() { throw 1; }
  
  int main() {
    std::set_terminate([]() {
      std::cout << "terminate called" << std::endl;
      std::abort();
    });
  
    f();
  }

However, in this case terminate is called using GCC 6.3 and Clang.

  #include <exception>
  #include <iostream>
   
  __attribute__((nothrow)) void f() { throw 1; }
   
  int main() {
    std::set_terminate([]() {
      std::cout << "terminate called" << std::endl;
      std::abort();
    });
   
    f();
  }

For your test case: GCC accepts (with __attribute__), MSVC accepts (with __declspec), and EDG rejects `Derived() noexcept = default` because of incompatible exception specs. I think it's probably reasonable for us to accept despite the slight semantic differences.

@STL_MSFT -- do you think `__declspec(nothrow)` calling the terminate handler in Clang is a bug?



================
Comment at: lib/Sema/SemaDeclCXX.cpp:170
 
+  if (EST == EST_None && Method->hasAttr<NoThrowAttr>()) {
+    EST = EST_BasicNoexcept;
----------------
Elide the braces.


================
Comment at: test/SemaCXX/nothrow-as-noexcept-ctor.cpp:3
+
+
+// expected-no-diagnostics
----------------
You can remove the spurious newline from the test.


https://reviews.llvm.org/D38209