[Clang] Check for abstract parameters only when functions are defined.

[cor3ntin]

Summary

The C++ standard allows abstract parameters in deleted functions
and in function declarations

The type of a parameter or the return type for a function definition
shall not be a (possibly cv-qualified) class type that is
incomplete or abstract within the function body
unless the function is deleted.

Fixes https://github.com/llvm/llvm-project/issues/63012

[aaron.ballman]

clang/lib/Sema/SemaDeclCXX.cpp

6089

This doesn't seem quite right -- we skip bodies for various reasons: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Parse/Parser.h#L413

Which test case needed this change?

clang/test/CXX/class.derived/class.abstract/p3.cpp

38–42

Whole lotta trailing semi colons that should be removed (you should get all of them, not just theses).

76–77

FIXME can be removed now? Do we need a DR test case for this?

clang/test/CXX/drs/dr6xx.cpp

707–708

Is this fixme still accurate? Should we be changing the resolution from partial to full?

clang/test/SemaCXX/abstract.cpp

369

A mildly horrifying test case to consider:

struct S {
  virtual void func() = 0;
};
void S::func() {}

static_assert(__is_abstract(S));

struct T {
  void func(S) = delete;
  void other(S);
  void yet_another(S) {}
};

We currently don't do good things... https://godbolt.org/z/7vEY3Kz9c

[cor3ntin]

clang/test/CXX/drs/dr6xx.cpp

707–708

nah, cf below (L714)

[cor3ntin]

Address Aaron's comments.

Initially these changes were tracked in
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0929r2.html

I fixed the specific github issue but more work is needed to acertain conformance.

[aaron.ballman]

LGTM, though please wait a day or so to land this in case @rjmccall disagrees with my assessment of the Objective-C changes.

clang/test/SemaObjCXX/parameters.mm

14–17

I think this change makes sense for Objective-C as well (this is a declaration and not a definition, at least as I understand things), but pinging @rjmccall just in case I'm wrong.

[rjmccall]

clang/test/SemaObjCXX/parameters.mm

14–17

No, that's right, methods in @interfaces and @protocols are just declarations.

Consistency with the base standard seems like the right way to go here, even those I personally find this rule rather silly. (It's one thing to allow this for deleted methods, but allowing it for declarations?)

clang/docs/ReleaseNotes.rst

 - Fix handling of generic lambda used as template arguments.
   (`#62611 <https://github.com/llvm/llvm-project/issues/62611>`_)
 - Allow omitting ``typename`` in the parameter declaration of a friend
   constructor declaration.
   (`#63119 <https://github.com/llvm/llvm-project/issues/63119>`_)
 - Fix access of a friend class declared in a local class. Clang previously
   emitted an error when a friend of a local class tried to access it's
   private data members.
+- Allow abstract parameter and return types in functions that are
+  either deleted or not defined.
+  (`#63012 <https://github.com/llvm/llvm-project/issues/63012>`_)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Preserve ``namespace`` definitions that follow malformed declarations.
 
 Miscellaneous Bug Fixes
 ^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaChecking.cpp

bool Sema::CheckParmsForFunctionDef(ArrayRef<ParmVarDecl *> Parameters,
                                     bool CheckParameterNames) {
   bool HasInvalidParm = false;
   for (ParmVarDecl *Param : Parameters) {
     assert(Param && "null in a parameter list");
     // C99 6.7.5.3p4: the parameters in a parameter type list in a
     // function declarator that is part of a function definition of
     // that function shall not have incomplete type.
     //
-    // This is also C++ [dcl.fct]p6.
+    // C++23 [dcl.fct.def.general]/p2
+    // The type of a parameter [...] for a function definition
+    // shall not be a (possibly cv-qualified) class type that is incomplete
+    // or abstract within the function body unless the function is deleted.
     if (!Param->isInvalidDecl() &&
-        RequireCompleteType(Param->getLocation(), Param->getType(),
-                            diag::err_typecheck_decl_incomplete_type)) {
+        (RequireCompleteType(Param->getLocation(), Param->getType(),
+                             diag::err_typecheck_decl_incomplete_type) ||
+         RequireNonAbstractType(Param->getBeginLoc(), Param->getOriginalType(),
+                                diag::err_abstract_type_in_decl,
+                                AbstractParamType))) {
       Param->setInvalidDecl();
       HasInvalidParm = true;
     }
 
     // C99 6.9.1p5: If the declarator includes a parameter type list, the
     // declaration of each parameter shall include an identifier.
     if (CheckParameterNames && Param->getIdentifier() == nullptr &&
         !Param->isImplicit() && !getLangOpts().CPlusPlus) {

clang/lib/Sema/SemaDecl.cpp

if (ConstexprKind == ConstexprSpecKind::Constinit) {
     SemaRef.Diag(D.getDeclSpec().getConstexprSpecLoc(),
                  diag::err_constexpr_wrong_decl_kind)
         << static_cast<int>(ConstexprKind);
     ConstexprKind = ConstexprSpecKind::Unspecified;
     D.getMutableDeclSpec().ClearConstexprSpec();
   }
   Expr *TrailingRequiresClause = D.getTrailingRequiresClause();
 
-  // 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(
-          D.getIdentifierLoc(), R->castAs<FunctionType>()->getReturnType(),
-          diag::err_abstract_type_in_decl, SemaRef.AbstractReturnType))
-    D.setInvalidType();
-
   if (Name.getNameKind() == DeclarationName::CXXConstructorName) {
     // This is a C++ constructor declaration.
     assert(DC->isRecord() &&
            "Constructors can only be declared in a member context");
 
     R = SemaRef.CheckConstructorDeclarator(D, R, SC);
     return CXXConstructorDecl::Create(
         SemaRef.Context, cast<CXXRecordDecl>(DC), D.getBeginLoc(), NameInfo, R,
if (New->isParameterPack())
     if (auto *LSI = getEnclosingLambda())
       LSI->LocalPacks.push_back(New);
 
   if (New->getType().hasNonTrivialToPrimitiveDestructCUnion() ||
       New->getType().hasNonTrivialToPrimitiveCopyCUnion())
     checkNonTrivialCUnion(New->getType(), New->getLocation(),
                           NTCUC_FunctionParam, NTCUK_Destruct|NTCUK_Copy);
 
-  // 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,
-                             AbstractParamType))
-    New->setInvalidDecl();
-
   // Parameter declarators cannot be interface types. All ObjC objects are
   // passed by reference.
   if (T->isObjCObjectType()) {
     SourceLocation TypeEndLoc =
         getLocForEndOfToken(TSInfo->getTypeLoc().getEndLoc());
     Diag(NameLoc,
          diag::err_object_cannot_be_passed_returned_by_value) << 1 << T
       << FixItHint::CreateInsertion(TypeEndLoc, "*");
Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
   if (unsigned BuiltinID = FD->getBuiltinID()) {
     if (!Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID) &&
         !Context.BuiltinInfo.isPredefinedRuntimeFunction(BuiltinID)) {
       Diag(FD->getLocation(), diag::err_builtin_definition) << FD;
       FD->setInvalidDecl();
     }
   }
 
-  // The return type of a function definition must be complete (C99 6.9.1p3),
-  // unless the function is deleted (C++ specifc, C++ [dcl.fct.def.general]p2)
+  // The return type of a function definition must be complete (C99 6.9.1p3).
+  // C++23 [dcl.fct.def.general]/p2
+  // The type of [...] the return for a function definition
+  // shall not be a (possibly cv-qualified) class type that is incomplete
+  // or abstract within the function body unless the function is deleted.
   QualType ResultType = FD->getReturnType();
   if (!ResultType->isDependentType() && !ResultType->isVoidType() &&
       !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
-      RequireCompleteType(FD->getLocation(), ResultType,
-                          diag::err_func_def_incomplete_result))
+      (RequireCompleteType(FD->getLocation(), ResultType,
+                           diag::err_func_def_incomplete_result) ||
+       RequireNonAbstractType(FD->getLocation(), FD->getReturnType(),
+                              diag::err_abstract_type_in_decl,
+                              AbstractReturnType)))
     FD->setInvalidDecl();
 
   if (FnBodyScope)
     PushDeclContext(FnBodyScope, FD);
 
   // Check the validity of our function parameters
   if (BodyKind != FnBodyKind::Delete)
     CheckParmsForFunctionDef(FD->parameters(),

clang/lib/Sema/SemaDeclCXX.cpp

void AbstractUsageInfo::CheckType(const NamedDecl *D, TypeLoc TL,
   CheckAbstractUsage(*this, D).Visit(TL, Sel);
 }
 
 }
 
 /// Check for invalid uses of an abstract type in a function declaration.
 static void CheckAbstractClassUsage(AbstractUsageInfo &Info,
                                     FunctionDecl *FD) {
-  // No need to do the check on definitions, which require that
-  // the return/param types be complete.
-  if (FD->doesThisDeclarationHaveABody())
+  // Only definitions are required to refer to complete and
+  // non-abstract types.
+  if (!FD->doesThisDeclarationHaveABody())
     return;
 
   // For safety's sake, just ignore it if we don't have type source
   // information.  This should never happen for non-implicit methods,
   // but...
   if (TypeSourceInfo *TSI = FD->getTypeSourceInfo())
     Info.CheckType(FD, TSI->getTypeLoc(), Sema::AbstractNone);
 }

clang/test/CXX/class.derived/class.abstract/p3.cpp

 };
 using B = SecretlyAbstract<int>;
 using C = SecretlyAbstract<float>;
 using D = SecretlyAbstract<char>[1];
 
 B b; // expected-error {{abstract class}}
 D d; // expected-error {{abstract class}}
 
-template<int> struct N;
+template<int> struct N{};
 
 // Note: C is not instantiated anywhere in this file, so we never discover that
 // it is in fact abstract. The C++ standard suggests that we need to
 // instantiate in all cases where abstractness could affect the validity of a
 // program, but that breaks a *lot* of code, so we don't do that.
 //
 // FIXME: Once DR1640 is resolved, remove the check on forming an abstract
 // array type entirely. The only restriction we need is that you can't create
 // an object of abstract (most-derived) type.
 
 
 // An abstract class shall not be used
 
 //  - as a parameter type
 void f(A&);
-void f(A); // expected-error {{abstract class}}
-void f(A[1]); // expected-error {{abstract class}}
-void f(B); // expected-error {{abstract class}}
-void f(B[1]); // expected-error {{abstract class}}
+void f(A){} // expected-error {{abstract class}}
+void f(A[1]){} // expected-error {{abstract class}}
+void f(B){} // expected-error {{abstract class}}
+void f(B[1]){} // expected-error {{abstract class}}
 void f(C);
 void f(C[1]);
-void f(D); // expected-error {{abstract class}}
-void f(D[1]); // expected-error {{abstract class}}
+void f(D){} // expected-error {{abstract class}}
+void f(D[1]){} // expected-error {{abstract class}}
 
 //  - as a function return type
 A &f(N<0>);
 A *f(N<1>);
-A f(N<2>); // expected-error {{abstract class}}
+A f(N<2>){} // expected-error {{abstract class}}
 A (&f(N<3>))[2]; // expected-error {{abstract class}}
-B f(N<4>); // expected-error {{abstract class}}
+B f(N<4>){} // expected-error {{abstract class}}
 B (&f(N<5>))[2]; // expected-error {{abstract class}}
 C f(N<6>);
 C (&f(N<7>))[2];
 
 //  - as the type of an explicit conversion
 void g(A&&);
 void h() {
   A(); // expected-error {{abstract class}}
   A(0); // expected-error {{abstract class}}
   A{}; // expected-error {{abstract class}}
   A{0}; // expected-error {{abstract class}}
   (A)(0); // expected-error {{abstract class}}
   (A){}; // expected-error {{abstract class}}
   (A){0}; // expected-error {{abstract class}}
 
   D(); // expected-error {{array type}}
   D{}; // expected-error {{abstract class}}
   D{0}; // expected-error {{abstract class}}
   (D){}; // expected-error {{abstract class}}
   (D){0}; // expected-error {{abstract class}}
 }
 
-template<typename T> void t(T); // expected-note 2{{abstract class}}
+template<typename T> void t(T);
 void i(A &a, B &b, C &c, D &d) {
-  // FIXME: These should be handled consistently. We currently reject the first
-  // two early because we (probably incorrectly, depending on dr1640) take
-  // abstractness into account in forming implicit conversion sequences.
-  t(a); // expected-error {{no matching function}}
-  t(b); // expected-error {{no matching function}}
+  t(a); // expected-error {{allocating an object of abstract class type 'A'}}
+  t(b); // expected-error {{allocating an object of abstract class type 'SecretlyAbstract<int>'}}
   t(c); // expected-error {{allocating an object of abstract class type}}
   t(d); // ok, decays to pointer
 }
 
 struct E : A {
   E() : A() {} // ok
   E(int n) : A( A(n) ) {} // expected-error {{abstract class}}
 };
 
 namespace std {
   template<typename T> struct initializer_list {
     const T *begin, *end;
     initializer_list();
   };
 }
 std::initializer_list<A> ila = {1, 2, 3, 4}; // expected-error {{abstract class}}

clang/test/CXX/drs/dr6xx.cpp

void f() {
     accept<const A&>(x);
     accept<const A&>(y); // expected-error {{private base class}}
     accept<const A&>(vy); // expected-error {{call to deleted}} expected-error {{no matching constructor}}
     accept<const D&>(c);
   }
 }
 
 namespace dr657 { // dr657: partial
-  struct Abs { virtual void x() = 0; };
+  struct Abs { virtual void x() = 0; }; // expected-note {{unimplemented pure virtual method 'x' in 'Abs'}}
   struct Der : public Abs { virtual void x(); };
 
   struct Cnvt { template<typename F> Cnvt(F); };
 
   void foo(Cnvt a);
   void foo(Abs &a);
   void f(Abs *a) { foo(*a); }
 
   void bar(Abs &a);
   template<typename T> void bar(T);
   void g(Abs *a) { bar(*a); }
 
   // FIXME: The following examples demonstrate that we might be accepting the
   // above cases for the wrong reason.
 
-  // FIXME: We should reject this.
-  struct C { C(Abs) {} };
-  // FIXME: We should reject this.
-  struct Q { operator Abs() { __builtin_unreachable(); } } q;
+  struct C { C(Abs) {} }; // expected-error {{parameter type 'Abs' is an abstract class}}
+  struct Q { operator Abs() { __builtin_unreachable(); } } q; // expected-error {{return type 'Abs' is an abstract class}}
 #if __cplusplus >= 201703L
   // FIXME: We should *definitely* reject this.
   C c = Q().operator Abs();
 #endif
 
   template<typename F> struct Cnvt2 { Cnvt2(F); typedef int type; };
 
   // FIXME: We should reject this.

clang/test/SemaCXX/abstract.cpp

class E : D {
   virtual void f();
 };
 
 static_assert(!__is_abstract(E), "E inherits from an abstract class but implements f");
 
 C *d = new C; // expected-error {{allocating an object of abstract class type 'C'}}
 
 C c; // expected-error {{variable type 'C' is an abstract class}}
-void t1(C c); // expected-error {{parameter type 'C' is an abstract class}}
-void t2(C); // expected-error {{parameter type 'C' is an abstract class}}
+void t1(C c);
+void t2(C);
+void t3(C c){} // expected-error {{parameter type 'C' is an abstract class}}
+void t4(C){} // expected-error {{parameter type 'C' is an abstract class}}
 
 struct S {
   C c; // expected-error {{field type 'C' is an abstract class}}
 };
 
-void t3(const C&);
+void t5(const C&);
 
 void f() {
   C(); // expected-error {{allocating an object of abstract class type 'C'}}
-  t3(C()); // expected-error {{allocating an object of abstract class type 'C'}}
+  t5(C()); // expected-error {{allocating an object of abstract class type 'C'}}
 }
 
 C e1[2]; // expected-error {{array of abstract class type 'C'}}
 C (*e2)[2]; // expected-error {{array of abstract class type 'C'}}
 C (**e3)[2]; // expected-error {{array of abstract class type 'C'}}
 
-void t4(C c[2]); // expected-error {{array of abstract class type 'C'}}
+void t6(C c[2]); // expected-error {{array of abstract class type 'C'}}
 
-void t5(void (*)(C)); // expected-error {{parameter type 'C' is an abstract class}}
+void t7(void (*)(C));
 
-typedef void (*Func)(C); // expected-error {{parameter type 'C' is an abstract class}}
-void t6(Func);
+typedef void (*Func)(C);
+void t8(Func);
 
 class F {
   F a() { while (1) {} } // expected-error {{return type 'F' is an abstract class}}
-    
   class D {
-    void f(F c); // expected-error {{parameter type 'F' is an abstract class}}
+    void f(F c){}  // expected-error {{parameter type 'F' is an abstract class}}
+    void g(F c);
+    void h(F c) = delete;
   };
 
   union U {
-    void u(F c); // expected-error {{parameter type 'F' is an abstract class}}
+    void u(F c){} // expected-error {{parameter type 'F' is an abstract class}}
+    void v(F c);
+    void w(F c) = delete;
   };
-    
   virtual void f() = 0; // expected-note {{unimplemented pure virtual method 'f'}}
 };
 
 // Diagnosing in these cases is prohibitively expensive.  We still
 // diagnose at the function definition, of course.
 
 class Abstract;
 
-void t7(Abstract a);
+void t8(Abstract a);
 
-void t8() {
+void t9() {
   void h(Abstract a);
 }
 
 namespace N {
 void h(Abstract a);
 }
 
 class Abstract {
namespace PR5222 {
   };
   struct B : public A {
     virtual B *clone() = 0;
   };
   struct C : public B {
     virtual C *clone();
   };
 
-  C c;  
+  C c;
 }
 
 // PR5550 - instantiating template didn't track overridden methods
 namespace PR5550 {
   struct A {
     virtual void a() = 0;
     virtual void b() = 0;
   };
   template<typename T> struct B : public A {
     virtual void b();
     virtual void c() = 0;
   };
   struct C : public B<int> {
     virtual void a();
     virtual void c();
-  }; 
+  };
   C x;
 }
 
 namespace PureImplicit {
   // A pure virtual destructor should be implicitly overridden.
   struct A { virtual ~A() = 0; };
   struct B : A {};
   B x;
void test() {
     C c;
   }
 }
 
 // rdar://problem/8302168
 namespace test2 {
   struct X1 {
     virtual void xfunc(void) = 0;  // expected-note {{unimplemented pure virtual method}}
-    void g(X1 parm7);        // expected-error {{parameter type 'test2::X1' is an abstract class}}
-    void g(X1 parm8[2]);     // expected-error {{array of abstract class type 'test2::X1'}}
+    void g(X1 parm7){}        // expected-error {{parameter type 'X1' is an abstract class}}
+    void g(X1 parm8[2]){}     // expected-error {{parameter type 'X1' is an abstract class}}
   };
 
   template <int N>
   struct X2 {
     virtual void xfunc(void) = 0;  // expected-note {{unimplemented pure virtual method}}
-    void g(X2 parm10);        // expected-error {{parameter type 'X2<N>' is an abstract class}}
-    void g(X2 parm11[2]);     // expected-error {{array of abstract class type 'X2<N>'}}
+    void g(X2 parm10){}        // expected-error {{parameter type 'X2<N>' is an abstract class}}
+    void g(X2 parm11[2]) {}     // expected-error {{parameter type 'X2<N>' is an abstract class}}
   };
 }
 
 namespace test3 {
   struct A { // expected-note {{not complete until}}
     A x; // expected-error {{field has incomplete type}}
     virtual void abstract() = 0;
   };

 
 struct var_template_def { // expected-note {{until the closing '}'}}
   template<typename T>
   static inline var_template_def v = {}; // expected-error {{incomplete type}} expected-warning 2{{extension}}
   virtual void f() = 0;
 };
 
 struct friend_fn {
-  friend void g(friend_fn); // expected-error {{abstract class}}
-  virtual void f() = 0; // expected-note {{unimplemented}}
+  friend void g(friend_fn);
+  virtual void f() = 0;
 };
 
 struct friend_fn_def {
   friend void g(friend_fn_def) {} // expected-error {{abstract class}}
   virtual void f() = 0; // expected-note {{unimplemented}}
 };
 
 struct friend_template {
   template<typename T>
-  friend void g(friend_template); // expected-error {{abstract class}}
-  virtual void f() = 0; // expected-note {{unimplemented}}
+  friend void g(friend_template);
+  virtual void f() = 0;
 };
 
 struct friend_template_def {
   template<typename T>
   friend void g(friend_template_def) {} // expected-error {{abstract class}}
   virtual void f() = 0; // expected-note {{unimplemented}}
 };
+
+namespace GH63012 {
+struct foo {
+    virtual ~foo() = 0;
+};
+void f(foo) = delete;
+foo  i() = delete;
+void h(foo);
+foo  g();
+
+struct S {
+  virtual void func() = 0; // expected-note {{unimplemented pure virtual method 'func' in 'S'}}
+};
+void S::func() {}
+
+static_assert(__is_abstract(S), "");
+
+struct T {
+  void func(S) = delete;
+  void other(S);
+  void yet_another(S) {} // expected-error{{parameter type 'S' is an abstract class}}
+};
+
+}

clang/test/SemaCXX/auto-type-from-cxx.cpp

int d() {
   struct BitField { int field:2; };
   __auto_type h = BitField{1}.field; // (should work from C++)
   new __auto_type; // expected-error {{'__auto_type' not allowed in type allocated by 'new'}}
 }
 
 namespace TestDeductionFail {
 
 template<typename T>
-void caller(T x) {x.fun();} // expected-note {{candidate template ignored: substitution failure [with T = TestDeductionFail::Abstract]: parameter type 'TestDeductionFail::Abstract' is an abstract class}}
+void caller(T x) {x.fun();} // expected-error {{parameter type 'TestDeductionFail::Abstract' is an abstract class}}
 
 template<typename T>
 auto getCaller(){
-  return caller<T>; // expected-error {{cannot deduce return type 'auto' from returned value of type '<overloaded function type>'}}
+  return caller<T>; // expected-note {{in instantiation of function template specialization 'TestDeductionFail::caller<TestDeductionFail::Abstract>' requested here}}
 }
 
 class Abstract{
   public:
     void fun();
-    virtual void vfun()=0;
-    void call(){getCaller<Abstract>()(*this);} // expected-note {{in instantiation of function template specialization 'TestDeductionFail::getCaller<TestDeductionFail::Abstract>' requested here}}
+    virtual void vfun()=0; // expected-note {{unimplemented pure virtual method 'vfun' in 'Abstract'}}
+    void call(){getCaller<Abstract>()(*this);} // expected-error {{allocating an object of abstract class type 'TestDeductionFail::Abstract'}}
 };
 
 }

clang/test/SemaObjCXX/parameters.mm

 // RUN: %clang_cc1 -verify %s
 
 @interface A
 @end
 
 template<typename T>
 struct X0 {
   void f(T); // expected-error{{interface type 'A' cannot be passed by value}}
 };
 
 X0<A> x0a; // expected-note{{instantiation}}
 
 
-struct test2 { virtual void foo() = 0; }; // expected-note {{unimplemented}}
+struct test2 { virtual void foo() = 0; };
 @interface Test2
-- (void) foo: (test2) foo; // expected-error {{parameter type 'test2' is an abstract class}}
+- (void) foo: (test2) foo ;
 @end
 
 template<typename T> void r1(__restrict T);
 void r2(__restrict id x) { r1(x); }