arphaman updated this revision to Diff 75403.
arphaman added a comment.

The updated patch improves error handling and adds a test for the fixit.

> If we issue a fixit we should recover as-if the code was written with the 
> fixit in. Does this code do that? (can we test it? I know we test some fixits 
> - not sure it's necessary/worthwhile to test them all, but maybe we have a 
> good idiom for testing that the recovery is correct)

This code does perform recovery, but the constructed AST for the destructor 
calls is different from the AST that would have been constructed if the code 
was correct: we still end up building the pseudo destructor expression. I'm not 
sure how important is that though, so please let me know if I should try and 
make the ASTs the same.


Repository:
  rL LLVM

https://reviews.llvm.org/D25817

Files:
  lib/Sema/SemaExprCXX.cpp
  test/CXX/special/class.dtor/p10-0x.cpp
  test/FixIt/fixit.cpp
  test/SemaCXX/pseudo-destructors.cpp

Index: test/SemaCXX/pseudo-destructors.cpp
===================================================================
--- test/SemaCXX/pseudo-destructors.cpp
+++ test/SemaCXX/pseudo-destructors.cpp
@@ -89,3 +89,26 @@
 void AliasTemplate(int *p) {
   p->~Id<int>();
 }
+
+namespace dotPointerAccess {
+struct Base {
+  virtual ~Base() {}
+};
+
+struct Derived : Base {
+  ~Derived() {}
+};
+
+void test() {
+  Derived d;
+  static_cast<Base *>(&d).~Base(); // expected-error {{member reference type 'dotPointerAccess::Base *' is a pointer; did you mean to use '->'}}
+  d->~Derived(); // expected-error {{member reference type 'dotPointerAccess::Derived' is not a pointer; did you mean to use '.'}}
+}
+
+typedef Derived *Foo;
+
+void test2(Foo d) {
+  d.~Foo(); // This is ok
+  d.~Derived(); // expected-error {{member reference type 'Foo' (aka 'dotPointerAccess::Derived *') is a pointer; did you mean to use '->'}}
+}
+}
Index: test/FixIt/fixit.cpp
===================================================================
--- test/FixIt/fixit.cpp
+++ test/FixIt/fixit.cpp
@@ -395,3 +395,14 @@
 // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:26-[[@LINE-1]]:26}:"{}"
 int use_czi = czi.a;
 
+namespace dotPointerDestructor {
+
+struct Bar {
+  ~Bar();
+};
+
+void bar(Bar *o) {
+  o.~Bar(); // expected-error {{member reference type 'dotPointerAccess::Base *' is a pointer; did you mean to use '->'}}
+}  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:4-[[@LINE-1]]:5}:"->"
+
+}
Index: test/CXX/special/class.dtor/p10-0x.cpp
===================================================================
--- test/CXX/special/class.dtor/p10-0x.cpp
+++ test/CXX/special/class.dtor/p10-0x.cpp
@@ -33,7 +33,7 @@
                              expected-error{{the type of object expression ('int') does not match the type being destroyed ('decltype(intp())' (aka 'int *')) in pseudo-destructor expression}}
   i.~decltype(intp())(); // expected-error{{the type of object expression ('int') does not match the type being destroyed ('decltype(intp())' (aka 'int *')) in pseudo-destructor expression}}
   pi->~decltype(int())();
-  pi.~decltype(int())(); // expected-error{{the type of object expression ('int *') does not match the type being destroyed ('decltype(int())' (aka 'int')) in pseudo-destructor expression}}
+  pi.~decltype(int())(); // expected-error{{member reference type 'int *' is a pointer; did you mean to use '->'?}}
   pi.~decltype(intp())();
   pi->~decltype(intp())(); // expected-error{{the type of object expression ('int') does not match the type being destroyed ('decltype(intp())' (aka 'int *')) in pseudo-destructor expression}}
 }
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -6275,15 +6275,31 @@
       = DestructedTypeInfo->getTypeLoc().getLocalSourceRange().getBegin();
     if (!DestructedType->isDependentType() && !ObjectType->isDependentType()) {
       if (!Context.hasSameUnqualifiedType(DestructedType, ObjectType)) {
-        Diag(DestructedTypeStart, diag::err_pseudo_dtor_type_mismatch)
-          << ObjectType << DestructedType << Base->getSourceRange()
-          << DestructedTypeInfo->getTypeLoc().getLocalSourceRange();
-
-        // Recover by setting the destructed type to the object type.
-        DestructedType = ObjectType;
-        DestructedTypeInfo = Context.getTrivialTypeSourceInfo(ObjectType,
-                                                           DestructedTypeStart);
-        Destructed = PseudoDestructorTypeStorage(DestructedTypeInfo);
+        // Detect dot pseudo destructor calls on pointer objects, e.g.:
+        //   Foo *foo;
+        //   foo.~Foo();
+        if (OpKind == tok::period && ObjectType->isPointerType() &&
+            Context.hasSameUnqualifiedType(DestructedType,
+                                           ObjectType->getPointeeType())) {
+          Diag(OpLoc, diag::err_typecheck_member_reference_suggestion)
+              << ObjectType << /*IsArrow=*/0 << Base->getSourceRange()
+              << FixItHint::CreateReplacement(OpLoc, "->");
+
+          // Recover by setting the object type to the destructed type and the
+          // operator to '->'.
+          ObjectType = DestructedType;
+          OpKind = tok::arrow;
+        } else {
+          Diag(DestructedTypeStart, diag::err_pseudo_dtor_type_mismatch)
+              << ObjectType << DestructedType << Base->getSourceRange()
+              << DestructedTypeInfo->getTypeLoc().getLocalSourceRange();
+
+          // Recover by setting the destructed type to the object type.
+          DestructedType = ObjectType;
+          DestructedTypeInfo =
+              Context.getTrivialTypeSourceInfo(ObjectType, DestructedTypeStart);
+          Destructed = PseudoDestructorTypeStorage(DestructedTypeInfo);
+        }
       } else if (DestructedType.getObjCLifetime() !=
                                                 ObjectType.getObjCLifetime()) {
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to