Skip to content
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

[clang]Avoid diagnose invalid consteval call for invalid function decl #68646

Merged
merged 3 commits into from
Oct 10, 2023
Merged

[clang]Avoid diagnose invalid consteval call for invalid function decl #68646

merged 3 commits into from
Oct 10, 2023

Conversation

HerrCai0907
Copy link
Contributor

Fixes:#68542
It is meaningless to diagnose further error for a invalid function declaration.

Fixes:#68542
It is meaningless to diagnoise further error for a invalid function declaration.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 10, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 10, 2023

@llvm/pr-subscribers-clang

Author: Congcong Cai (HerrCai0907)

Changes

Fixes:#68542
It is meaningless to diagnose further error for a invalid function declaration.


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

2 Files Affected:

  • (modified) clang/lib/Sema/SemaExpr.cpp (+2-1)
  • (added) clang/test/SemaCXX/PR68542.cpp (+20)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 9c5f96eebd04165..2e01b82b13d819e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18406,9 +18406,10 @@ static void EvaluateAndDiagnoseImmediateInvocation(
       FD = Call->getConstructor();
     else if (auto *Cast = dyn_cast<CastExpr>(InnerExpr))
       FD = dyn_cast_or_null<FunctionDecl>(Cast->getConversionFunction());
-
     assert(FD && FD->isImmediateFunction() &&
            "could not find an immediate function in this expression");
+    if (FD->isInvalidDecl())
+      return;
     SemaRef.Diag(CE->getBeginLoc(), diag::err_invalid_consteval_call)
         << FD << FD->isConsteval();
     if (auto Context =
diff --git a/clang/test/SemaCXX/PR68542.cpp b/clang/test/SemaCXX/PR68542.cpp
new file mode 100644
index 000000000000000..bc94fffe5de289d
--- /dev/null
+++ b/clang/test/SemaCXX/PR68542.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+
+// expected-note@+2{{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'S &&' for 1st argument}}
+// expected-note@+1{{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const S &' for 1st argument}}
+struct S {
+    int e;
+};
+
+template<class T>
+consteval int get_format() {
+	return nullptr; // expected-error{{cannot initialize return object of type 'int' with an rvalue of type 'std::nullptr_t'}}
+}
+
+template<class T>
+constexpr S f(T) noexcept {
+	return get_format<T>(); // expected-error{{no viable conversion from returned value of type 'int' to function return type 'S'}}
+}
+
+// expected-note@+1{{in instantiation of function template specialization 'f<int>' requested here}}
+constexpr S x = f(0); // expected-error{{constexpr variable 'x' must be initialized by a constant expression}}
\ No newline at end of file

Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

Needs a release note, plus a couple nits. Otherwise LGTM!

clang/test/SemaCXX/PR68542.cpp Outdated Show resolved Hide resolved
clang/test/SemaCXX/PR68542.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaExpr.cpp Show resolved Hide resolved
@@ -358,6 +358,8 @@ Bug Fixes in This Version
Fixes (`#67690 <https://github.com/llvm/llvm-project/issues/67690>`_)
- Fixes a ``clang-17`` regression where ``LLVM_UNREACHABLE_OPTIMIZE=OFF``
cannot be used with ``Release`` mode builds. (`#68237 <https://github.com/llvm/llvm-project/issues/68237>`_).
- Fix crash in evaluating ``constexpr`` value for invalid template function.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Fix crash in evaluating ``constexpr`` value for invalid template function.
- Fix crash when evaluating an invalid immediate function template.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

I was about to work on that and figured the fix would be along those lines! Glad you got to it first, thanks!

The changes look great

@HerrCai0907 HerrCai0907 merged commit 19d1da5 into llvm:main Oct 10, 2023
3 checks passed
@HerrCai0907 HerrCai0907 deleted the fix/68542 branch October 10, 2023 07:32
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
Development

Successfully merging this pull request may close these issues.

Clang crashes when a consteval function has multiple diagnostics in flight
4 participants