[Clang] Qualified functions can't decay into pointers

[MitalAshok]

Fixes #27059

Dependent function parameters, template parameters and exception declarations that have qualified function types now error instead of silently decaying into an invalid pointer type.

Also fix __decay and __add_pointer for these types which previously just ignored the qualifiers

[llvmbot]

@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)

Changes

Fixes #27059

Dependent function parameters, template parameters and exception declarations that have qualified function types now error instead of silently decaying into an invalid pointer type.

Also fix __decay and __add_pointer for these types which previously just ignored the qualifiers

---

Full diff: https://github.com/llvm/llvm-project/pull/90353.diff

10 Files Affected:

• (modified) clang/include/clang/AST/Type.h (+3-1)
• (modified) clang/lib/AST/TypePrinter.cpp (+23)
• (modified) clang/lib/Sema/SemaDecl.cpp (+7)
• (modified) clang/lib/Sema/SemaDeclCXX.cpp (+13-3)
• (modified) clang/lib/Sema/SemaTemplate.cpp (+22-2)
• (modified) clang/lib/Sema/SemaType.cpp (+16-30)
• (modified) clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6-0x.cpp (+2-3)
• (modified) clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6.cpp (+2-3)
• (modified) clang/test/SemaCXX/function-type-qual.cpp (+35-1)
• (modified) clang/test/SemaCXX/type-traits.cpp (+12)

diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index dff02d4861b3db..bf8187385f062c 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -5035,6 +5035,8 @@ class FunctionProtoType final
     return static_cast<RefQualifierKind>(FunctionTypeBits.RefQualifier);
   }
 
+  std::string getFunctionQualifiersAsString() const;
+
   using param_type_iterator = const QualType *;
 
   ArrayRef<QualType> param_types() const {
@@ -7370,7 +7372,7 @@ inline bool QualType::isReferenceable() const {
   if (const auto *F = Self.getAs<FunctionProtoType>())
     return F->getMethodQuals().empty() && F->getRefQualifier() == RQ_None;
 
-  return false;
+  return Self.isFunctionType();
 }
 
 inline SplitQualType QualType::split() const {
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 9602f448e94279..720ac254539f11 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2571,3 +2571,26 @@ raw_ostream &clang::operator<<(raw_ostream &OS, QualType QT) {
   TypePrinter(LangOptions()).print(S.Ty, S.Quals, OS, /*PlaceHolder=*/"");
   return OS;
 }
+
+std::string FunctionProtoType::getFunctionQualifiersAsString() const {
+  std::string Quals = getMethodQuals().getAsString();
+
+  switch (getRefQualifier()) {
+  case RQ_None:
+    break;
+
+  case RQ_LValue:
+    if (!Quals.empty())
+      Quals += ' ';
+    Quals += '&';
+    break;
+
+  case RQ_RValue:
+    if (!Quals.empty())
+      Quals += ' ';
+    Quals += "&&";
+    break;
+  }
+
+  return Quals;
+}
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e0745fe9a45367..66748fd9ca17f6 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15392,6 +15392,13 @@ ParmVarDecl *Sema::CheckParameter(DeclContext *DC, SourceLocation StartLoc,
     T = Context.getLifetimeQualifiedType(T, lifetime);
   }
 
+  if (T->isFunctionType() && !T.isReferenceable()) {
+    Diag(NameLoc, diag::err_compound_qualified_function_type)
+        << 1 << true << T
+        << T->castAs<FunctionProtoType>()->getFunctionQualifiersAsString();
+    return nullptr;
+  }
+
   ParmVarDecl *New = ParmVarDecl::Create(Context, DC, StartLoc, NameLoc, Name,
                                          Context.getAdjustedParameterType(T),
                                          TSInfo, SC, nullptr);
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index abdbc9d8830c03..d5630b609cabc4 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -11323,7 +11323,8 @@ void Sema::CheckConversionDeclarator(Declarator &D, QualType &R,
     D.setInvalidType();
   } else if (ConvType->isFunctionType()) {
     Diag(D.getIdentifierLoc(), diag::err_conv_function_to_function);
-    ConvType = Context.getPointerType(ConvType);
+    if (ConvType.isReferenceable())
+      ConvType = Context.getPointerType(ConvType);
     D.setInvalidType();
   }
 
@@ -16974,8 +16975,17 @@ VarDecl *Sema::BuildExceptionDeclaration(Scope *S, TypeSourceInfo *TInfo,
   // Arrays and functions decay.
   if (ExDeclType->isArrayType())
     ExDeclType = Context.getArrayDecayedType(ExDeclType);
-  else if (ExDeclType->isFunctionType())
-    ExDeclType = Context.getPointerType(ExDeclType);
+  else if (ExDeclType->isFunctionType()) {
+    if (ExDeclType.isReferenceable())
+      ExDeclType = Context.getPointerType(ExDeclType);
+    else {
+      Diag(Loc, diag::err_compound_qualified_function_type)
+          << 1 << true << ExDeclType
+          << ExDeclType->castAs<FunctionProtoType>()
+                 ->getFunctionQualifiersAsString();
+      Invalid = true;
+    }
+  }
 
   // C++ 15.3p1: The exception-declaration shall not denote an incomplete type.
   // The exception-declaration shall not denote a pointer or reference to an
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index bbcb7c33a98579..bb60ea31e51625 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1472,8 +1472,16 @@ QualType Sema::CheckNonTypeTemplateParameterType(QualType T,
   //   A non-type template-parameter of type "array of T" or
   //   "function returning T" is adjusted to be of type "pointer to
   //   T" or "pointer to function returning T", respectively.
-  if (T->isArrayType() || T->isFunctionType())
+  if (T->isArrayType() || T->isFunctionType()) {
+    if (!T.isReferenceable()) {
+      // Pointer to cv- or ref- qualified type will be invalid
+      Diag(Loc, diag::err_compound_qualified_function_type)
+          << 1 << true << T
+          << T->castAs<FunctionProtoType>()->getFunctionQualifiersAsString();
+      return QualType();
+    }
     return Context.getDecayedType(T);
+  }
 
   // If T is a dependent type, we can't do the check now, so we
   // assume that it is well-formed. Note that stripping off the
@@ -2679,8 +2687,20 @@ struct ConvertConstructorToDeductionGuideTransform {
     }
     // Handle arrays and functions decay.
     auto NewType = NewDI->getType();
-    if (NewType->isArrayType() || NewType->isFunctionType())
+    if (NewType->isArrayType())
       NewType = SemaRef.Context.getDecayedType(NewType);
+    else if (NewType->isFunctionType()) {
+      // Reject cv- and ref-qualified function
+      if (!NewType.isReferenceable()) {
+        SemaRef.Diag(OldParam->getLocation(),
+                     diag::err_compound_qualified_function_type)
+            << 1 << true << NewType
+            << cast<FunctionProtoType>(NewType)
+                   ->getFunctionQualifiersAsString();
+        return nullptr;
+      }
+      NewType = SemaRef.Context.getDecayedType(NewType);
+    }
 
     ParmVarDecl *NewParam = ParmVarDecl::Create(
         SemaRef.Context, DC, OldParam->getInnerLocStart(),
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index fddc3545ecb61c..9647f856f7d244 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -2118,29 +2118,6 @@ static QualType inferARCLifetimeForPointee(Sema &S, QualType type,
   return S.Context.getQualifiedType(type, qs);
 }
 
-static std::string getFunctionQualifiersAsString(const FunctionProtoType *FnTy){
-  std::string Quals = FnTy->getMethodQuals().getAsString();
-
-  switch (FnTy->getRefQualifier()) {
-  case RQ_None:
-    break;
-
-  case RQ_LValue:
-    if (!Quals.empty())
-      Quals += ' ';
-    Quals += '&';
-    break;
-
-  case RQ_RValue:
-    if (!Quals.empty())
-      Quals += ' ';
-    Quals += "&&";
-    break;
-  }
-
-  return Quals;
-}
-
 namespace {
 /// Kinds of declarator that cannot contain a qualified function type.
 ///
@@ -2166,8 +2143,8 @@ static bool checkQualifiedFunction(Sema &S, QualType T, SourceLocation Loc,
     return false;
 
   S.Diag(Loc, diag::err_compound_qualified_function_type)
-    << QFK << isa<FunctionType>(T.IgnoreParens()) << T
-    << getFunctionQualifiersAsString(FPT);
+      << QFK << isa<FunctionType>(T.IgnoreParens()) << T
+      << FPT->getFunctionQualifiersAsString();
   return true;
 }
 
@@ -2178,7 +2155,7 @@ bool Sema::CheckQualifiedFunctionForTypeId(QualType T, SourceLocation Loc) {
     return false;
 
   Diag(Loc, diag::err_qualified_function_typeid)
-      << T << getFunctionQualifiersAsString(FPT);
+      << T << FPT->getFunctionQualifiersAsString();
   return true;
 }
 
@@ -3091,7 +3068,16 @@ QualType Sema::BuildFunctionType(QualType T,
 
   for (unsigned Idx = 0, Cnt = ParamTypes.size(); Idx < Cnt; ++Idx) {
     // FIXME: Loc is too inprecise here, should use proper locations for args.
-    QualType ParamType = Context.getAdjustedParameterType(ParamTypes[Idx]);
+    QualType ParamType = ParamTypes[Idx];
+    if (ParamType->isFunctionType() && !ParamType.isReferenceable()) {
+      Diag(Loc, diag::err_compound_qualified_function_type)
+          << 1 << true << ParamType
+          << ParamType->castAs<FunctionProtoType>()
+                 ->getFunctionQualifiersAsString();
+      Invalid = true;
+    } else
+      ParamType = Context.getAdjustedParameterType(ParamType);
+
     if (ParamType->isVoidType()) {
       Diag(Loc, diag::err_param_with_void_type);
       Invalid = true;
@@ -5979,9 +5965,9 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
       }
 
       S.Diag(Loc, diag::err_invalid_qualified_function_type)
-        << Kind << D.isFunctionDeclarator() << T
-        << getFunctionQualifiersAsString(FnTy)
-        << FixItHint::CreateRemoval(RemovalRange);
+          << Kind << D.isFunctionDeclarator() << T
+          << FnTy->getFunctionQualifiersAsString()
+          << FixItHint::CreateRemoval(RemovalRange);
 
       // Strip the cv-qualifiers and ref-qualifiers from the type.
       FunctionProtoType::ExtProtoInfo EPI = FnTy->getExtProtoInfo();
diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6-0x.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6-0x.cpp
index d93cc8b90874d5..8f01406f8bae55 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6-0x.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6-0x.cpp
@@ -53,11 +53,10 @@ void (X::*mpf2)() && = &X::f1;
 
 void (f() &&); // expected-error{{non-member function cannot have '&&' qualifier}}
 
-// FIXME: These are ill-formed.
 template<typename T> struct pass {
-  void f(T);
+  void f(T); // expected-error {{pointer to function type cannot have '&' qualifier}}
 };
-pass<func_type_lvalue> pass0;
+pass<func_type_lvalue> pass0; // expected-note {{in instantiation of template class 'pass<void () &>' requested here}}
 pass<func_type_lvalue> pass1;
 
 template<typename T, typename U> struct is_same { static const bool value = false; };
diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6.cpp
index a035086c9a127e..82b2f8102217ed 100644
--- a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p6.cpp
@@ -26,8 +26,7 @@ template<typename T> struct S {
 };
 S<F> s; // expected-note {{in instantiation of}}
 
-// FIXME: This is ill-formed.
 template<typename T> struct U {
-  void f(T);
+  void f(T); // expected-error {{pointer to function type cannot have 'const' qualifier}}
 };
-U<F> u;
+U<F> u; // expected-note {{in instantiation of}}
diff --git a/clang/test/SemaCXX/function-type-qual.cpp b/clang/test/SemaCXX/function-type-qual.cpp
index f4906f58abbae0..aaf91aa6b9a182 100644
--- a/clang/test/SemaCXX/function-type-qual.cpp
+++ b/clang/test/SemaCXX/function-type-qual.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify %s
 
 void f() const; // expected-error {{non-member function cannot have 'const' qualifier}}
 void (*pf)() const; // expected-error {{pointer to function type cannot have 'const' qualifier}}
@@ -7,6 +7,9 @@ extern void (&rf)() const; // expected-error {{reference to function type cannot
 typedef void cfn() const;
 cfn f2; // expected-error {{non-member function of type 'cfn' (aka 'void () const') cannot have 'const' qualifier}}
 
+void decay1(void p() const); // expected-error {{non-member function cannot have 'const' qualifier}}
+void decay2(cfn p); // expected-error {{non-member function of type 'cfn' (aka 'void () const') cannot have 'const' qualifier}}
+
 class C {
   void f() const;
   cfn f2;
@@ -55,3 +58,34 @@ struct B {
   void operator delete[](void*) volatile; //expected-error {{static member function cannot have 'volatile' qualifier}}
 };
 }
+
+namespace GH27059 {
+template<typename T> int f(T); // #GH27059-f
+template<typename T, T> int g(); // #GH27059-g
+int x = f<void () const>(nullptr);
+// expected-error@-1 {{no matching function for call to 'f'}}
+//   expected-note@#GH27059-f {{candidate template ignored: substitution failure [with T = void () const]: pointer to function type cannot have 'const' qualifier}}
+int y = g<void () const, nullptr>();
+// expected-error@-1 {{no matching function for call to 'g'}}
+//   expected-note@#GH27059-g {{invalid explicitly-specified argument for 2nd template parameter}}
+
+template<typename T> int ff(void p(T)); // #GH27059-ff
+template<typename T, void(T)> int gg(); // #GH27059-gg
+int xx = ff<void () const>(nullptr);
+// expected-error@-1 {{no matching function for call to 'ff'}}
+//   expected-note@#GH27059-ff {{candidate template ignored: substitution failure [with T = void () const]: pointer to function type cannot have 'const' qualifier}}
+int yy = gg<void () const, nullptr>();
+// expected-error@-1 {{no matching function for call to 'gg'}}
+//   expected-note@#GH27059-gg {{invalid explicitly-specified argument for 2nd template parameter}}
+
+template<typename T>
+void catch_fn() {
+  try {
+  } catch (T) { // #GH27059-catch_fn
+  }
+}
+template void catch_fn<void()>();
+template void catch_fn<void() const>();
+// expected-error@#GH27059-catch_fn {{pointer to function type cannot have 'const' qualifier}}
+//   expected-note@-2 {{in instantiation of function template specialization 'GH27059::catch_fn<void () const>' requested here}}
+}
diff --git a/clang/test/SemaCXX/type-traits.cpp b/clang/test/SemaCXX/type-traits.cpp
index dee4a29bd2bffe..5c3e03f09de227 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -4111,6 +4111,12 @@ void add_pointer() {
   static_assert(__is_same(add_pointer_t<int()>, int (*)()));
   static_assert(__is_same(add_pointer_t<int (*)()>, int (**)()));
   static_assert(__is_same(add_pointer_t<int (&)()>, int (*)()));
+  static_assert(__is_same(add_pointer_t<int () const>, int () const));
+  static_assert(__is_same(add_pointer_t<int () &>, int () &));
+  static_assert(__is_same(add_pointer_t<int[]>, int(*)[]));
+  static_assert(__is_same(add_pointer_t<int[1]>, int(*)[1]));
+  static_assert(__is_same(add_pointer_t<int(&)[1]>, int(*)[1]));
+  static_assert(__is_same(add_pointer_t<int(&&)[1]>, int(*)[1]));
 
   static_assert(__is_same(add_pointer_t<S>, S *));
   static_assert(__is_same(add_pointer_t<const S>, const S *));
@@ -4315,6 +4321,12 @@ void check_decay() {
   static_assert(__is_same(decay_t<int (&)()>, int (*)()));
   static_assert(__is_same(decay_t<IntAr>, int *));
   static_assert(__is_same(decay_t<IntArNB>, int *));
+  static_assert(__is_same(decay_t<int () const>, int () const));
+  static_assert(__is_same(decay_t<int () &>, int () &));
+  static_assert(__is_same(decay_t<int[]>, int*));
+  static_assert(__is_same(decay_t<int[1]>, int*));
+  static_assert(__is_same(decay_t<int(&)[1]>, int*));
+  static_assert(__is_same(decay_t<int(&&)[1]>, int*));
 
   static_assert(__is_same(decay_t<S>, S));
   static_assert(__is_same(decay_t<S &>, S));

[cor3ntin]

Thanks!
I think the general approach makes sense

There are a few DR that might be worth mentioning/add tests for/look into

https://cplusplus.github.io/CWG/issues/713.html
https://cplusplus.github.io/CWG/issues/1417.html
https://cplusplus.github.io/CWG/issues/1584.html

[cor3ntin]

We have similar logic thorough this PR, having a function like DiagnoseUnreferenceableFunctionTypes would be nice

[MitalAshok]

https://cplusplus.github.io/CWG/issues/713.html

This one is entirely editorial: it adds a note and edits another note. I've added a test for what the note says

https://cplusplus.github.io/CWG/issues/1417.html

I'm not sure that this DR actually changes anything. It seems pointers/references to function pointers were ill-formed before and remain ill-formed afterwards.

https://cplusplus.github.io/CWG/issues/1584.html

Clang 7 implemented the 2015 resolution of this: 413f3c5

[Endilll]

Current CWG713 behavior was introduced sometime after Clang 18, and is not affected by this patch.
Current CWG1584 behavior was introduced in Clang 7, and is also not affected by this patch. (https://godbolt.org/z/xYec9rP5x)
This PR is large enough already. I suggest to move those tests out to a separate PR.