Sema: Implement DR244

[majnemer]

Summary

Naming the destructor using a typedef-name for the class-name is
well-formed.

This fixes PR19620.

[majnemer]

Add a missing expected-no-diagnostics annotation.

[rsmith]

lib/Sema/SemaExprCXX.cpp

132–133

Please update this to match the standard (in particular, "the second class-name is looked up in the same scope as the first").

154–161

By a strict reading of the standard, I think:

namespace N {
  struct A {
    struct B {};
  };
  A *a;
  A::B *b;
}
void f() {
  a->N::~A(); // ok
  b->N::A::~B(); // ill-formed, B is *only* looked up in N
}

... but see also core issue 399. I think our behavior here (with your patch) is appropriate for now.

test/CXX/drs/dr2xx.cpp

1016

Why was this deleted? If we no longer reject this, that's a regression, and we can no longer claim to implement DR298...

[majnemer]

lib/Sema/SemaExprCXX.cpp

132–133

Done. :)

test/CXX/drs/dr2xx.cpp

1016

It was deleted because it conflicts with the following line. Not only do we still reject, we provide a superior diagnostic :)

I've updated the test-case so that we test both cases.

[majnemer]

Address @rsmith's review comments.

[rsmith]

Looks great, thanks!

[majnemer]

Closed by commit rL209319 (authored by @majnemer).

[jwillemsen]

This fix doesn't seem to be part of clang 3.5, see http://llvm.org/bugs/show_bug.cgi?id=18879

[jwillemsen]

Doesn't seem to be fixed in 3.6, any idea when this gets fixed in master?

[jwillemsen]

This isn't fixed in 3.6.2, any idea when this gets into a release?

[rsmith]

This will be in 3.7.

[jwillemsen]

Just tested 3.7.0, this issue is not fixed in this new release of today

[jwillemsen]

In D3583#222045, @rsmith wrote:

This will be in 3.7.

As far as we can tell DR244 is not fixed yet in 3.9.1 so any idea why this review sets it to YES as implemented?

lib/Sema/SemaExprCXX.cpp

ParsedType Sema::getDestructorName(SourceLocation TildeLoc,
   if (ObjectTypePtr)
     SearchType = GetTypeFromParser(ObjectTypePtr);
 
   if (SS.isSet()) {
     NestedNameSpecifier *NNS = SS.getScopeRep();
 
     bool AlreadySearched = false;
     bool LookAtPrefix = true;
-    // C++ [basic.lookup.qual]p6:
+    // C++11 [basic.lookup.qual]p6:
     //   If a pseudo-destructor-name (5.2.4) contains a nested-name-specifier,
     //   the type-names are looked up as types in the scope designated by the
-    //   nested-name-specifier. In a qualified-id of the form:
+    //   nested-name-specifier. Similarly, in a qualified-id of the form:
     //
-    //     ::[opt] nested-name-specifier  ~ class-name
+    //     nested-name-specifier[opt] class-name :: ~ class-name
     //
-    //   where the nested-name-specifier designates a namespace scope, and in
-    //   a qualified-id of the form:
+    //   the second class-name is looked up in the same scope as the first.
     //
-    //     ::opt nested-name-specifier class-name ::  ~ class-name
-    //
-    //   the class-names are looked up as types in the scope designated by
-    //   the nested-name-specifier.
-    //
-    // Here, we check the first case (completely) and determine whether the
-    // code below is permitted to look at the prefix of the
-    // nested-name-specifier.
+    // Here, we determine whether the code below is permitted to look at the
+    // prefix of the nested-name-specifier.
     DeclContext *DC = computeDeclContext(SS, EnteringContext);
     if (DC && DC->isFileContext()) {
       AlreadySearched = true;
       LookupCtx = DC;
       isDependent = false;
-    } else if (DC && isa<CXXRecordDecl>(DC))
+    } else if (DC && isa<CXXRecordDecl>(DC)) {
       LookAtPrefix = false;
+      LookInScope = true;
+    }
 
     // The second case from the C++03 rules quoted further above.
     NestedNameSpecifier *Prefix = 0;
     if (AlreadySearched) {
       // Nothing left to do.
     } else if (LookAtPrefix && (Prefix = NNS->getPrefix())) {
       CXXScopeSpec PrefixSS;
       PrefixSS.Adopt(NestedNameSpecifierLoc(Prefix, SS.location_data()));
       LookupCtx = computeDeclContext(PrefixSS, EnteringContext);
       isDependent = isDependentScopeSpecifier(PrefixSS);
     } else if (ObjectTypePtr) {
       LookupCtx = computeDeclContext(SearchType);
       isDependent = SearchType->isDependentType();
     } else {
       LookupCtx = computeDeclContext(SS, EnteringContext);
       isDependent = LookupCtx && LookupCtx->isDependentContext();
     }
-
-    LookInScope = false;
   } else if (ObjectTypePtr) {
     // C++ [basic.lookup.classref]p3:
     //   If the unqualified-id is ~type-name, the type-name is looked up
     //   in the context of the entire postfix-expression. If the type T
     //   of the object expression is of a class type C, the type-name is
     //   also looked up in the scope of class C. At least one of the
     //   lookups shall find a name that refers to (possibly
     //   cv-qualified) T.
     LookupCtx = computeDeclContext(SearchType);

test/CXX/basic/basic.lookup/basic.lookup.qual/p6.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
 
 struct C { 
   typedef int I;
 }; 
 
 typedef int I1, I2; 
 extern int* p; 
 extern int* q; 
 
 void f() {
   p->C::I::~I(); 
   q->I1::~I2();
 }
 
 struct A { 
   ~A();
 }; 
 
 typedef A AB; 
 int main() {
   AB *p; 
-  p->AB::~AB(); // expected-error{{expected the class name after '~' to name a destructor}}
+  p->AB::~AB();
 }

test/CXX/drs/dr2xx.cpp

namespace dr243 { // dr243: yes
   };
   struct B {
     operator A() = delete; // expected-error 0-1{{extension}} expected-note {{candidate}}
   } b;
   A a1(b);
   A a2 = b; // expected-error {{ambiguous}}
 }
 
-namespace dr244 { // dr244: no
+namespace dr244 { // dr244: 3.5
   struct B {}; struct D : B {}; // expected-note {{here}}
 
   D D_object;
   typedef B B_alias;
   B* B_ptr = &D_object;
 
   void f() {
     D_object.~B(); // expected-error {{expression does not match the type}}
     D_object.B::~B();
     B_ptr->~B();
     B_ptr->~B_alias();
     B_ptr->B_alias::~B();
     // This is valid under DR244.
-    B_ptr->B_alias::~B_alias(); // FIXME: expected-error {{expected the class name after '~' to name a destructor}}
+    B_ptr->B_alias::~B_alias();
     B_ptr->dr244::~B(); // expected-error {{refers to a member in namespace}}
     B_ptr->dr244::~B_alias(); // expected-error {{refers to a member in namespace}}
   }
 }
 
 namespace dr245 { // dr245: yes
   struct S {
     enum E {}; // expected-note {{here}}
namespace dr298 { // dr298: yes
   C::type i3;
 
   struct A a;
   struct B b; // expected-error {{refers to a typedef}}
   struct C c; // expected-error {{refers to a typedef}}
 
   B::B() {} // expected-error {{requires a type specifier}}
   B::A() {} // ok
-  C::~C() {} // expected-error {{expected the class name}}
-  C::~A() {} // ok
+  C::~C() {} // expected-error {{destructor cannot be declared using a typedef 'C' (aka 'const dr298::A') of the class name}}
 
   typedef struct D E; // expected-note {{here}}
   struct E {}; // expected-error {{conflicts with typedef}}
+
+  struct F {
+    ~F();
+  };
+  typedef const F G;
+  G::~F() {} // ok
 }
 
 namespace dr299 { // dr299: yes c++11
   struct S {
     operator int();
   };
   struct T {
     operator int(); // expected-note {{}}
     operator unsigned short(); // expected-note {{}}
   };
   // FIXME: should this apply to c++98 mode?
   int *p = new int[S()]; // expected-error 0-1{{extension}}
   int *q = new int[T()]; // expected-error {{ambiguous}}
 }

www/cxx_dr_status.html

<tr id="243">
     <td>NAD</td>
     <td>Weighting of conversion functions in direct-initialization</td>
     <td class="full" align="center">Yes</td>
   </tr>
   <tr id="244">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#244">244</a></td>
     <td>CD1</td>
     <td>Destructor lookup</td>
-    <td class="none" align="center">No</td>
+    <td class="svn" align="center">SVN</td>
   </tr>
   <tr id="245">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#245">245</a></td>
     <td>CD1</td>
     <td>Name lookup in <I>elaborated-type-specifier</I>s</td>
     <td class="full" align="center">Yes</td>
   </tr>
   <tr id="246">