[clang][Sema] deleted overriding function can have lax except spec

[Lancern]

According to CWG1351, overriding functions that are defined as deleted can have more lax exception specifications compared to the base version. For example, the following code should compile:

struct B {
  virtual void h() noexcept = delete;
};

struct D: B {
  void h() override = delete;  // Should be OK
};

Clang incorrectly reports that the exception specification of D::h is more lax than base version. This patch fixes this.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Sirui Mu (Lancern)

Changes

According to CWG1351, overriding functions that are defined as deleted can have more lax exception specifications compared to the base version. For example, the following code should compile:

struct B {
  virtual void h() noexcept = delete;
};

struct D: B {
  void h() override = delete;  // Should be OK
};

Clang incorrectly reports that the exception specification of D::h is more lax than base version. This patch fixes this.

---

Full diff: https://github.com/llvm/llvm-project/pull/76248.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaExceptionSpec.cpp (+5)
• (modified) clang/test/CXX/except/except.spec/p5-virtual.cpp (+4)

diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 75730ea888afb4..f8f9c9d491a18e 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -979,6 +979,11 @@ bool Sema::CheckOverridingFunctionExceptionSpec(const CXXMethodDecl *New,
   if (isa<CXXDestructorDecl>(New) && New->getParent()->isDependentType())
     return false;
 
+  // CWG1351: if either of the old function or the new function is defined as
+  // deleted, we don't need this check.
+  if (Old->isDeleted() || New->isDeleted())
+    return false;
+
   // If the old exception specification hasn't been parsed yet, or the new
   // exception specification can't be computed yet, remember that we need to
   // perform this check when we get to the end of the outermost
diff --git a/clang/test/CXX/except/except.spec/p5-virtual.cpp b/clang/test/CXX/except/except.spec/p5-virtual.cpp
index 69daec6ee53347..2cecd6ce51f698 100644
--- a/clang/test/CXX/except/except.spec/p5-virtual.cpp
+++ b/clang/test/CXX/except/except.spec/p5-virtual.cpp
@@ -45,6 +45,8 @@ struct Base
   virtual void f15();
   virtual void f16();
 
+  virtual void f17() noexcept = delete;
+
   virtual void g1() throw(); // expected-note {{overridden virtual function is here}}
   virtual void g2() throw(int); // expected-note {{overridden virtual function is here}}
   virtual void g3() throw(A); // expected-note {{overridden virtual function is here}}
@@ -81,6 +83,8 @@ struct Derived : Base
   virtual void f15() noexcept;
   virtual void f16() throw();
 
+  virtual void f17() = delete;
+
   virtual void g1() throw(int); // expected-error {{exception specification of overriding function is more lax}}
   virtual void g2(); // expected-error {{exception specification of overriding function is more lax}}
   virtual void g3() throw(D); // expected-error {{exception specification of overriding function is more lax}}

[shafik]

Either they both have to be deleted or not. We should diagnose otherwise. So I think checking both is redundant.

[Lancern]

Oops, code formatter formats unrelated code. Revert and re-commit.

[erichkeane]

This needs a release note. Also, since this is implementing a core issue, this needs to update a DRs test as well.

[Endilll]

You need to run clang/www/make_cxx_drs script in order to make cxx_dr_status.html reflect that CWG1351 is now implemented.

[Endilll]

I guess you intended // dr1351: 18.