[clang] [Clang] Qualified functions can't decay into pointers (PR #90353)

Mital Ashok via cfe-commits cfe-commits at lists.llvm.org
Sat Jun 29 03:54:06 PDT 2024


https://github.com/MitalAshok updated https://github.com/llvm/llvm-project/pull/90353

>From b81ffefb52981276b70570f878e3e75e30a49fbb Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sat, 27 Apr 2024 19:15:00 +0100
Subject: [PATCH] [Clang] Qualified functions can't decay into pointers

---
 clang/docs/ReleaseNotes.rst                   |  3 ++
 clang/include/clang/AST/Type.h                |  4 +-
 clang/lib/AST/TypePrinter.cpp                 | 23 ++++++++++
 clang/lib/Sema/SemaDecl.cpp                   |  7 +++
 clang/lib/Sema/SemaDeclCXX.cpp                | 16 +++++--
 clang/lib/Sema/SemaTemplate.cpp               | 24 +++++++++-
 clang/lib/Sema/SemaType.cpp                   | 46 +++++++------------
 .../dcl.decl/dcl.meaning/dcl.fct/p6-0x.cpp    |  5 +-
 .../CXX/dcl.decl/dcl.meaning/dcl.fct/p6.cpp   |  5 +-
 clang/test/SemaCXX/function-type-qual.cpp     | 36 ++++++++++++++-
 clang/test/SemaCXX/type-traits.cpp            | 12 +++++
 11 files changed, 138 insertions(+), 43 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index feba3c7ba8d77..57caaff12d51b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -740,6 +740,9 @@ Bug Fixes in This Version
   negatives where the analysis failed to detect unchecked access to guarded
   data.
 
+- cv- and ref- qualified function types no longer silently produce invalid pointer to
+  qualified function types when they implicitly decay in some places. Fixes (#GH27059).
+
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 62836ec5c6312..b9c1fdd22ab94 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -5373,6 +5373,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 {
@@ -7761,7 +7763,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 7c87fd587880e..7acadf60f56dd 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -2602,3 +2602,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 029ccf944c513..69270c2f35656 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15370,6 +15370,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 ffa6bcac44cbd..c9f415c50062f 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -11301,7 +11301,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();
   }
 
@@ -16962,8 +16963,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 e36ee2d5a46cf..d8ae966289e73 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1447,8 +1447,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
@@ -2672,8 +2680,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 308274720d58d..5417fd69bc162 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -1706,29 +1706,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.
 ///
@@ -1754,8 +1731,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;
 }
 
@@ -1766,7 +1743,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;
 }
 
@@ -2679,7 +2656,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;
@@ -5540,9 +5526,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 d93cc8b90874d..8f01406f8bae5 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 a035086c9a127..82b2f8102217e 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 f4906f58abbae..aaf91aa6b9a18 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 at -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 at -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 at -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 at -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 at -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 d40605f56f1ed..99845b463abed 100644
--- a/clang/test/SemaCXX/type-traits.cpp
+++ b/clang/test/SemaCXX/type-traits.cpp
@@ -4223,6 +4223,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 *));
@@ -4427,6 +4433,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));



More information about the cfe-commits mailing list