Fix for PR18393 - emit error when abstract type is returned or taken by value

[matekm]

Summary

Hi,

Attached patch contains the implementation of a fix for PR18393[1]. According to standard "An abstract class shall not be used as a parameter type, as a function return type, or as the type of an explicit conversion" (class.abstract $10.4.3).

Currently, checking if type isn't abstract class is done when method is defined, but I don't see any reason why clang shouldn't do it as early as possible, in this case, when function/method is declared. Test also attached.

Please, review and provide feedback or propose additional tests.

Thanks in advance
Robert Matusewicz

[rsmith]

Clang doesn't do this immediately because that's not correct:

struct S {
  S f();
  virtual void g() = 0;
};

We can't tell that we need to diagnose f until we reach the declaration
of g.

[matekm]

In D5409#4, @rsmith wrote:

Clang doesn't do this immediately because that's not correct:

struct S {
  S f();
  virtual void g() = 0;
};

We can't tell that we need to diagnose f until we reach the declaration
of g.

Hi,

actually after applying my patch the case you presented is handled correctly. The problematic case is this one:

struct AbstractClass;

struct S {
  AbstractClass f();
};

struct AbstracClass {
   virtual void g() = 0;
};

This isn't catched by my patch. I will dig deeper to check where is the problem

[pipping]

Any updates on this?

lib/Sema/SemaDecl.cpp

   bool isExplicit = D.getDeclSpec().isExplicitSpecified();
   bool isConstexpr = D.getDeclSpec().isConstexprSpecified();
 
-  // Check that the return type is not an abstract class type.
-  // For record types, this is done by the AbstractClassUsageDiagnoser once
-  // the class has been completely parsed.
-  if (!DC->isRecord() &&
-      SemaRef.RequireNonAbstractType(
+  if (SemaRef.RequireNonAbstractType(
           D.getIdentifierLoc(), R->getAs<FunctionType>()->getReturnType(),
           diag::err_abstract_type_in_decl, SemaRef.AbstractReturnType))
     D.setInvalidType();

                                          TSInfo,
                                          StorageClass, nullptr);
 
-  // Parameters can not be abstract class types.
-  // For record types, this is done by the AbstractClassUsageDiagnoser once
-  // the class has been completely parsed.
-  if (!CurContext->isRecord() &&
-      RequireNonAbstractType(NameLoc, T, diag::err_abstract_type_in_decl,
+  if (RequireNonAbstractType(NameLoc, T, diag::err_abstract_type_in_decl,
                              AbstractParamType))
     New->setInvalidDecl();
 

test/SemaCXX/abstract.cpp

   };
 }
 
+namespace PR18393 {
+  struct A {
+      typedef A type;
+      virtual void f() = 0; // expected-note {{unimplemented pure virtual method 'f' in 'A'}}
+  };
+
+  struct B {
+      A f1(); // expected-error {{return type 'PR18393::A' is an abstract class}}
+
+      void f2(A a); // expected-error {{parameter type 'PR18393::A' is an abstract class}}
+  };
+
+  A f1(); // expected-error {{return type 'PR18393::A' is an abstract class}}
+
+  void f2(A a);  // expected-error {{parameter type 'PR18393::A' is an abstract class}}
+
+  template<typename T>
+  struct Sample {
+  public:
+      typename T::type abstract();
+      void abstract(typename T::type);
+  };
+}
+
 namespace pr12658 {
   class C {
     public: