Skip to content

[Clang] Diagnose invalid function types in dependent contexts #138731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 6, 2025

Conversation

cor3ntin
Copy link
Contributor

@cor3ntin cor3ntin commented May 6, 2025

When forming an invalid function type, we were not diagnosing it if the call was dependent.

However, we later rely on the function type to be sensible during argument deduction.

We now diagnose anything that is not a potential function type, to avoid constructing bogus call expressions.

Fixes #138657
Fixes #115725
Fixes #68852

When forming an invalid function type, we were not diagnosing
it if the call was dependent.

However, we later rely on the function type to be sensible
during argument deduction.

We now diagnose anything that is not a potential function type,
to avoid constructing bogus call expressions.

Fixes llvm#138657
Fixes llvm#115725
Fixes llvm#68852
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 6, 2025
@llvmbot
Copy link
Member

llvmbot commented May 6, 2025

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

When forming an invalid function type, we were not diagnosing it if the call was dependent.

However, we later rely on the function type to be sensible during argument deduction.

We now diagnose anything that is not a potential function type, to avoid constructing bogus call expressions.

Fixes #138657
Fixes #115725
Fixes #68852


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaExpr.cpp (+19)
  • (modified) clang/test/SemaTemplate/fun-template-def.cpp (+50-1)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 203958dab7430..bdd54f6a52b05 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -655,6 +655,7 @@ Bug Fixes to C++ Support
 - Fixed an assertion when trying to constant-fold various builtins when the argument
   referred to a reference to an incomplete type. (#GH129397)
 - Fixed a crash when a cast involved a parenthesized aggregate initialization in dependent context. (#GH72880)
+- Fixed a crash when forming an invalid function type in a dependent context. (#GH138657) (#GH115725) (#GH68852)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index be3f145f3c5f1..f463bffa83545 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6541,6 +6541,15 @@ ExprResult Sema::ActOnCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
   return Call;
 }
 
+// Any type that could be used to form a callable expression
+static bool MayBeFunctionType(const ASTContext &Context, QualType T) {
+  return T == Context.BoundMemberTy || T == Context.UnknownAnyTy ||
+         T == Context.BuiltinFnTy || T == Context.OverloadTy ||
+         T->isFunctionType() || T->isFunctionReferenceType() ||
+         T->isMemberFunctionPointerType() || T->isFunctionPointerType() ||
+         T->isBlockPointerType() || T->isRecordType();
+}
+
 ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
                                MultiExprArg ArgExprs, SourceLocation RParenLoc,
                                Expr *ExecConfig, bool IsExecConfig,
@@ -6594,6 +6603,16 @@ ExprResult Sema::BuildCallExpr(Scope *Scope, Expr *Fn, SourceLocation LParenLoc,
             *this, dyn_cast<UnresolvedMemberExpr>(Fn->IgnoreParens()),
             Fn->getBeginLoc());
 
+        if (!Fn->getType()->isDependentType()) {
+          // If the type of the function itself is not dependent
+          // check that it is a reasonable as a function, as type deduction
+          // later assume the CallExpr has a sensible TYPE.
+          if (!MayBeFunctionType(Context, Fn->getType()))
+            return ExprError(
+                Diag(LParenLoc, diag::err_typecheck_call_not_function)
+                << Fn->getType() << Fn->getSourceRange());
+        }
+
         return CallExpr::Create(Context, Fn, ArgExprs, Context.DependentTy,
                                 VK_PRValue, RParenLoc, CurFPFeatureOverrides());
       }
diff --git a/clang/test/SemaTemplate/fun-template-def.cpp b/clang/test/SemaTemplate/fun-template-def.cpp
index de77901b5b601..716296e72bc44 100644
--- a/clang/test/SemaTemplate/fun-template-def.cpp
+++ b/clang/test/SemaTemplate/fun-template-def.cpp
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 // Tests that dependent expressions are always allowed, whereas non-dependent
 // are checked as usual.
@@ -32,7 +33,7 @@ T f1(T t1, U u1, int i1, T** tpp)
   i1 = t1[u1];
   i1 *= t1;
 
-  i1(u1, t1); // error
+  i1(u1, t1); // expected-error {{called object type 'int' is not a function or function pointer}}
   u1(i1, t1);
 
   U u2 = (T)i1;
@@ -60,3 +61,51 @@ void f3() {
   f2<int*>(0);
   f2<int>(0); // expected-error {{no matching function for call to 'f2'}}
 }
+
+#if __cplusplus >= 202002L
+namespace GH138657 {
+template <auto V> // #gh138657-template-head
+class meta {};
+template<int N>
+class meta<N()> {}; // expected-error {{called object type 'int' is not a function or function point}}
+
+template<int N[1]>
+class meta<N()> {}; // expected-error {{called object type 'int *' is not a function or function point}}
+
+template<char* N>
+class meta<N()> {}; // expected-error {{called object type 'char *' is not a function or function point}}
+
+struct S {};
+template<S>
+class meta<S()> {}; // expected-error {{template argument for non-type template parameter is treated as function type 'S ()'}}
+                    // expected-note@#gh138657-template-head {{template parameter is declared here}}
+
+}
+
+namespace GH115725 {
+template<auto ...> struct X {};
+template<typename T, typename ...Ts> struct A {
+  template<Ts ...Ns, T *...Ps>
+  A(X<0(Ps)...>, Ts (*...qs)[Ns]);
+  // expected-error@-1{{called object type 'int' is not a function or function pointer}}
+
+};
+}
+
+namespace GH68852 {
+template <auto v>
+struct constexpr_value {
+  template <class... Ts>
+  constexpr constexpr_value<v(Ts::value...)> call(Ts...) {
+    //expected-error@-1 {{called object type 'int' is not a function or function pointer}}
+    return {};
+  }
+};
+
+template <auto v> constexpr static inline auto c_ = constexpr_value<v>{};
+// expected-note@-1 {{in instantiation of template}}
+auto k = c_<1>; // expected-note {{in instantiation of variable}}
+
+}
+
+#endif

// If the type of the function itself is not dependent
// check that it is a reasonable as a function, as type deduction
// later assume the CallExpr has a sensible TYPE.
if (!MayBeFunctionType(Context, Fn->getType()))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might suggest putting these in teh same 'if' to make it more obvious what is happening (or perhaps have MayBeFunctionType return true for a dependent type?

@cor3ntin cor3ntin merged commit cf9b4d1 into llvm:main May 6, 2025
10 of 11 checks passed
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…38731)

When forming an invalid function type, we were not diagnosing it if the
call was dependent.

However, we later rely on the function type to be sensible during
argument deduction.

We now diagnose anything that is not a potential function type, to avoid
constructing bogus call expressions.

Fixes llvm#138657
Fixes llvm#115725
Fixes llvm#68852
@yuxuanchen1997
Copy link
Member

yuxuanchen1997 commented May 8, 2025

Hi @cor3ntin,

This patch seems to have made expression SFINAEs in variable template partial specialization an hard error: https://godbolt.org/z/EosEEnMsf

Upstream clang rejects the program while clang-20, gcc, msvc accepted it. This change breaks folly invocability suite, which is used by large amount of currently working code.

Can we revert until this issue gets sorted out?

@ayermolo
Copy link
Contributor

ayermolo commented May 8, 2025

This is causing major breakages in our internal builds in multiple services, please revert.

mizvekov added a commit that referenced this pull request May 8, 2025
mizvekov added a commit that referenced this pull request May 8, 2025
@mizvekov
Copy link
Contributor

mizvekov commented May 8, 2025

This is reverted now.

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request May 9, 2025
…nt contexts (#138731)" (#139176)

This reverts commit cf9b4d1.

Causes breakages as reported here:
llvm/llvm-project#138731 (comment)
cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request May 9, 2025
llvm#138731 (comment)

A call expression might have been partially constructed,
in which case it will be a call-expressions (and its type
will not be that of a function)

To address that, we check that the expression might already be
a well-formed call
cor3ntin added a commit to cor3ntin/llvm-project that referenced this pull request May 9, 2025
llvm#138731 (comment)

A call expression might have been partially constructed,
in which case it will be a call-expressions (and its type
will not be that of a function)

To address that, we check that the expression might already be
a well-formed call
@cor3ntin
Copy link
Contributor Author

cor3ntin commented May 9, 2025

Sorry about that, folks. And thanks for the minimal repro, it's greatly appreciated.
I relanded the PR here #139246
I hope that resolves the issue you found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
6 participants