[PATCH] SemaChecking: return type validation for member functions

[Saleem Abdulrasool]

Hi rsmith,

Perform additional validation on the return type of a CXXMemberDeclExpr.
Previously, we did not verify that the returned type is concrete if it were a
RecordDecl.  If we are not dealing with a decltype expression, perform an
additional check to ensure that there are no unimplemented virtual methods in
the return type.

This addresses PR18393.

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

Files:
  lib/Sema/SemaExprCXX.cpp
  test/Analysis/PR18393.cpp
  test/SemaCXX/conditional-expr.cpp

Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4946,6 +4946,13 @@
   bool IsDecltype = ExprEvalContexts.back().IsDecltype;
   CXXDestructorDecl *Destructor = IsDecltype ? 0 : LookupDestructor(RD);
 
+  if (!IsDecltype && RD->isAbstract()) {
+    Diag(E->getExprLoc(), diag::err_abstract_type_in_decl)
+      << Sema::AbstractReturnType << RT->desugar();
+    DiagnoseAbstractType(RD);
+    return Owned(E);
+  }
+
   if (Destructor) {
     MarkFunctionReferenced(E->getExprLoc(), Destructor);
     CheckDestructorAccess(E->getExprLoc(), Destructor,
Index: test/Analysis/PR18393.cpp
===================================================================
--- /dev/null
+++ test/Analysis/PR18393.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// PR18393
+
+struct base {
+  virtual void method() = 0;
+  // expected-note@-1 {{unimplemented pure virtual method 'method' in 'base'}}
+};
+
+struct derived : base {
+  virtual void method();
+};
+
+struct holder {
+  holder() : d_() {}
+  base get() const { return d_; }
+  const derived d_;
+};
+
+void function(const base &);
+
+void test() {
+  holder h;
+  function(h.get());
+  // expected-error@-1 {{return type 'base' is an abstract class}}
+}
+
Index: test/SemaCXX/conditional-expr.cpp
===================================================================
--- test/SemaCXX/conditional-expr.cpp
+++ test/SemaCXX/conditional-expr.cpp
@@ -219,7 +219,9 @@
   // *must* create a separate temporary copy of class objects. This can only
   // be properly tested at runtime, though.
 
-  const Abstract &a = true ? static_cast<const Abstract&>(Derived1()) : 
Derived2(); // expected-error {{allocating an object of abstract class type 
'const Abstract'}}
+  const Abstract &a = true ? static_cast<const Abstract&>(Derived1()) : 
Derived2();
+  // expected-error@-1 {{allocating an object of abstract class type 'const 
Abstract'}}
+  // expected-error@-2 {{return type 'Abstract' is an abstract class}}
   true ? static_cast<const Abstract&>(Derived1()) : throw 3; // expected-error 
{{allocating an object of abstract class type 'const Abstract'}}
 }

Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4946,6 +4946,13 @@
   bool IsDecltype = ExprEvalContexts.back().IsDecltype;
   CXXDestructorDecl *Destructor = IsDecltype ? 0 : LookupDestructor(RD);
 
+  if (!IsDecltype && RD->isAbstract()) {
+    Diag(E->getExprLoc(), diag::err_abstract_type_in_decl)
+      << Sema::AbstractReturnType << RT->desugar();
+    DiagnoseAbstractType(RD);
+    return Owned(E);
+  }
+
   if (Destructor) {
     MarkFunctionReferenced(E->getExprLoc(), Destructor);
     CheckDestructorAccess(E->getExprLoc(), Destructor,
Index: test/Analysis/PR18393.cpp
===================================================================
--- /dev/null
+++ test/Analysis/PR18393.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// PR18393
+
+struct base {
+  virtual void method() = 0;
+  // expected-note@-1 {{unimplemented pure virtual method 'method' in 'base'}}
+};
+
+struct derived : base {
+  virtual void method();
+};
+
+struct holder {
+  holder() : d_() {}
+  base get() const { return d_; }
+  const derived d_;
+};
+
+void function(const base &);
+
+void test() {
+  holder h;
+  function(h.get());
+  // expected-error@-1 {{return type 'base' is an abstract class}}
+}
+
Index: test/SemaCXX/conditional-expr.cpp
===================================================================
--- test/SemaCXX/conditional-expr.cpp
+++ test/SemaCXX/conditional-expr.cpp
@@ -219,7 +219,9 @@
   // *must* create a separate temporary copy of class objects. This can only
   // be properly tested at runtime, though.
 
-  const Abstract &a = true ? static_cast<const Abstract&>(Derived1()) : Derived2(); // expected-error {{allocating an object of abstract class type 'const Abstract'}}
+  const Abstract &a = true ? static_cast<const Abstract&>(Derived1()) : Derived2();
+  // expected-error@-1 {{allocating an object of abstract class type 'const Abstract'}}
+  // expected-error@-2 {{return type 'Abstract' is an abstract class}}
   true ? static_cast<const Abstract&>(Derived1()) : throw 3; // expected-error {{allocating an object of abstract class type 'const Abstract'}}
 }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits