[PATCH] D53860: [SemaCXX] Don't check base's dtor is accessible

[Brian Gesiak via Phabricator via cfe-commits]

modocache created this revision.
modocache added reviewers: rsmith, rjmccall, doug.gregor, ahatanak.

In https://reviews.llvm.org/D45898?id=157373, @rsmith advises to move a
`checkMemberDestructor` into `InitListChecker::CheckStructUnionTypes`,
to check base classes for an accessible destructor when performing
aggregate initialization. However, in so doing, the following C++ code
now fails to compile:

  class Base { protected: ~Base() = default; };
  struct Derived : Base {};
  void foo() { Derived d = {}; }

GCC 8.2 and Clang 7.0.0 both treat this C++ code as valid, whereas
Clang trunk emits the following error diagnostic:

  <source>:3:27: error: temporary of type 'Base' has protected destructor
  void foo() { Derived d = {}; }
                            ^
  <source>:1:25: note: declared protected here
  class Base { protected: ~Base() = default; };
                          ^

I think Clang trunk is wrong about the validity of this C++ code: my
understanding of the C++17 standard is that the `Base` destructor need
only be accessible from within the context of `Derived`, and not at the
context in which `Derived` is aggregate initialized within the `foo`
function.

Add a test for the case above, and modify the destructor access check
within `InitListChecker::CheckStructUnionTypes` to check only that the
derived class has an accessible destructor.

Test Plan: `check-clang`


Repository:
  rC Clang

https://reviews.llvm.org/D53860

Files:
  lib/Sema/SemaInit.cpp
  test/SemaCXX/aggregate-initialization.cpp


Index: test/SemaCXX/aggregate-initialization.cpp
===================================================================
--- test/SemaCXX/aggregate-initialization.cpp
+++ test/SemaCXX/aggregate-initialization.cpp
@@ -219,7 +219,7 @@
      struct S0 { int f; ~S0() = delete; }; // expected-note {{'~S0' has been 
explicitly marked deleted here}}
 
     // Check destructor of base class.
-    struct S3 : S0 {};
+    struct S3 : S0 {}; // expected-note {{destructor of 'S3' is implicitly 
deleted because base class 'ElementDestructor::BaseDestructor::S0' has a 
deleted destructor}}
 
     void test3() {
       S3 s3 = {1}; // expected-error {{attempt to use a deleted function}}
@@ -233,4 +233,10 @@
   struct B { B(); A a; };
   struct C { B b; };
   C c = { B() };
+
+  // Zylong's destructor doesn't have to be accessible from the context of
+  // Yerbl's initialization.
+  struct Zylong { protected: ~Zylong(); };
+  struct Yerbl : Zylong {};
+  Yerbl y = {};
 }
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -1954,7 +1954,7 @@
     }
 
     if (!VerifyOnly)
-      if (hasAccessibleDestructor(Base.getType(), InitLoc, SemaRef)) {
+      if (hasAccessibleDestructor(DeclType, InitLoc, SemaRef)) {
         hadError = true;
         return;
       }



Index: test/SemaCXX/aggregate-initialization.cpp
===================================================================
--- test/SemaCXX/aggregate-initialization.cpp
+++ test/SemaCXX/aggregate-initialization.cpp
@@ -219,7 +219,7 @@
      struct S0 { int f; ~S0() = delete; }; // expected-note {{'~S0' has been explicitly marked deleted here}}
 
     // Check destructor of base class.
-    struct S3 : S0 {};
+    struct S3 : S0 {}; // expected-note {{destructor of 'S3' is implicitly deleted because base class 'ElementDestructor::BaseDestructor::S0' has a deleted destructor}}
 
     void test3() {
       S3 s3 = {1}; // expected-error {{attempt to use a deleted function}}
@@ -233,4 +233,10 @@
   struct B { B(); A a; };
   struct C { B b; };
   C c = { B() };
+
+  // Zylong's destructor doesn't have to be accessible from the context of
+  // Yerbl's initialization.
+  struct Zylong { protected: ~Zylong(); };
+  struct Yerbl : Zylong {};
+  Yerbl y = {};
 }
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -1954,7 +1954,7 @@
     }
 
     if (!VerifyOnly)
-      if (hasAccessibleDestructor(Base.getType(), InitLoc, SemaRef)) {
+      if (hasAccessibleDestructor(DeclType, InitLoc, SemaRef)) {
         hadError = true;
         return;
       }

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits