[PATCH] [ms-cxxabi] Don't do destructor check on declarations if the dtor is deleted

Hans Wennborg hans at chromium.org
Mon Dec 16 17:50:21 PST 2013


Hi rsmith, rnk,

We would previously emit redundant diagnostics for the following code:

  struct S {
    virtual ~S() = delete;
    void operator delete(void*, int);
    void operator delete(void*, double);
  } s;

First we would check on ~S() and error about the ambigous delete functions,
and then we would error about using the deleted destructor.

If the destructor is deleted, there's no need to check it.

Also, move the check from Sema::ActOnFields to CheckCompleteCXXClass. These
are run at almost the same time, called from ActOnFinishCXXMemberSpecification.
However, CHeckCompleteCXXClass may mark a defaulted destructor as deleted, and
if that's the case we don't want to check it.

There's a discussion about whether we should be doing these checks here at all,
but while we've got them, let's at least handle the case of deleted functions.

http://llvm-reviews.chandlerc.com/D2421

Files:
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp

Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -6332,7 +6332,7 @@
       // any translation unit may need to emit a deleting destructor.
       if (SemaRef.Context.getTargetInfo().getCXXABI().isMicrosoft() &&
           !Record->isDependentType() && Record->getDefinition() &&
-          !Record->isBeingDefined()) {
+          !Record->isBeingDefined() && !NewDD->isDeleted()) {
         SemaRef.CheckDestructor(NewDD);
       }
 
@@ -12082,12 +12082,6 @@
             if (getLangOpts().CPlusPlus11)
               AdjustDestructorExceptionSpec(CXXRecord,
                                             CXXRecord->getDestructor());
-
-            // The Microsoft ABI requires that we perform the destructor body
-            // checks (i.e. operator delete() lookup) at every declaration, as
-            // any translation unit may need to emit a deleting destructor.
-            if (Context.getTargetInfo().getCXXABI().isMicrosoft())
-              CheckDestructor(CXXRecord->getDestructor());
           }
 
           // Add any implicitly-declared members to this class.
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -4469,6 +4469,15 @@
         }
       }
     }
+
+    if (Record->hasUserDeclaredDestructor()) {
+      // The Microsoft ABI requires that we perform the destructor body
+      // checks (i.e. operator delete() lookup) at every declaration, as
+      // any translation unit may need to emit a deleting destructor.
+      if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+          !Record->getDestructor()->isDeleted())
+        CheckDestructor(Record->getDestructor());
+    }
   }
 
   // C++11 [dcl.constexpr]p8: A constexpr specifier for a non-static member
Index: test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/microsoft-dtor-lookup-cxx11.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple i686-pc-win32 -cxx-abi microsoft -std=c++11 -verify %s
+
+struct S {
+  virtual ~S() = delete; // expected-note {{function has been explicitly marked deleted here}}
+  void operator delete(void*, int);
+  void operator delete(void*, double);
+} s; // expected-error {{attempt to use a deleted function}}
+
+struct T { // expected-note{{virtual destructor requires an unambiguous, accessible 'operator delete'}}
+  virtual ~T() = default; // expected-note {{explicitly defaulted function was implicitly deleted here}}
+  void operator delete(void*, int);
+  void operator delete(void*, double);
+} t; // expected-error {{attempt to use a deleted function}}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2421.1.patch
Type: text/x-patch
Size: 2811 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131216/dcd5b07e/attachment.bin>


More information about the cfe-commits mailing list