[clang] e5f7123 - Disable constexpr function body checking in more situations (#94347)

[via cfe-commits]

Author: Aaron Ballman
Date: 2024-06-04T13:32:11-04:00
New Revision: e5f7123dfef3a80937d088d846ddb3b2bb1869b9

URL: https://github.com/llvm/llvm-project/commit/e5f7123dfef3a80937d088d846ddb3b2bb1869b9
DIFF: https://github.com/llvm/llvm-project/commit/e5f7123dfef3a80937d088d846ddb3b2bb1869b9.diff

LOG: Disable constexpr function body checking in more situations (#94347)

Before C++23, we would check a constexpr function body to diagnose if
the function can never be evaluated in a constant expression context.
This was previously required standards behavior, but C++23 relaxed the
restrictions with P2448R2. While this checking is useful, it is also
quite expensive, especially in pathological cases (see #92924 for an
example), because it means the mere presence of a constexpr function
definition will require constant evaluation even if the function is not
used within the TU.

Clang suppresses diagnostics in system headers by default and system
headers (like STL implementations) can be full of constexpr function
bodies. Now we suppress the check for a diagnostic if the function
definition is in a system header or if the `-Winvalid-constexpr`
diagnostic is disabled. This should have some mild compile time
performance improvements.

Also, the previous implementation would disable the diagnostic in C++23
mode entirely. Due to the benefit of the check, this patch now makes it
possible to enable the diagnostic explicitly in C++23 mode.

Added: 
    clang/test/SemaCXX/constexpr-never-constant.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/LangOptions.def
    clang/include/clang/Driver/Options.td
    clang/lib/Frontend/CompilerInvocation.cpp
    clang/lib/Sema/SemaDeclCXX.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 99580c0d28a4f..39a9013c75a41 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -207,6 +207,9 @@ C++23 Feature Support
 - Implemented `P1774R8: Portable assumptions <https://wg21.link/P1774R8>`_.
 
 - Implemented `P2448R2: Relaxing some constexpr restrictions <https://wg21.link/P2448R2>`_.
+  Note, the ``-Winvalid-constexpr`` diagnostic is now disabled in C++23 mode,
+  but can be explicitly specified to retain the old diagnostic checking
+  behavior.
 
 - Added a ``__reference_converts_from_temporary`` builtin, completing the necessary compiler support for
   `P2255R2: Type trait to determine if a reference binds to a temporary <https://wg21.link/P2255R2>`_.
@@ -323,6 +326,17 @@ Non-comprehensive list of changes in this release
 - Builtins ``__builtin_shufflevector()`` and ``__builtin_convertvector()`` may
   now be used within constant expressions.
 
+- When compiling a constexpr function, Clang will check to see whether the
+  function can *never* be used in a constant expression context and issues a
+  diagnostic under the ``-Winvalid-constexpr`` diagostic flag (which defaults
+  to an error). This check can be expensive because the mere presence of a
+  function marked ``constexpr`` will cause us to undergo constant expression
+  evaluation, even if the function is not called within the translation unit
+  being compiled. Due to the expense, Clang no longer checks constexpr function
+  bodies when the function is defined in a system header file or when
+  ``-Winvalid-constexpr`` is not enabled for the function definition, which
+  should result in mild compile-time performance improvements.
+
 New Compiler Flags
 ------------------
 - ``-fsanitize=implicit-bitfield-conversion`` checks implicit truncation and

diff  --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index 4061451b2150a..2dea3cd4d795b 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -505,6 +505,14 @@ COMPATIBLE_LANGOPT(IncrementalExtensions, 1, 0, " True if we want to process sta
 
 BENIGN_LANGOPT(CheckNew, 1, 0, "Do not assume C++ operator new may not return NULL")
 
+// FIXME: It would be better for us to find a way to encode the state of this
+// diagnostic in tablegen so that we can specify a particular diagnostic option
+// is disabled or enabled based on other language options or made it easier to
+// do this from the compiler invocation without hitting option round-tripping
+// issues.
+BENIGN_LANGOPT(CheckConstexprFunctionBodies, 1, 1,
+               "Emit diagnostics for a constexpr function body that can never "
+               "be used in a constant expression.")
 #undef LANGOPT
 #undef COMPATIBLE_LANGOPT
 #undef BENIGN_LANGOPT

diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 57f37c5023110..5ab2d49c7a497 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -557,6 +557,17 @@ multiclass BoolMOption<string flag_base, KeyPathAndMacro kpm,
               Group<m_Group>;
 }
 
+/// Creates a BoolOption where both of the flags are prefixed with "W", are in
+/// the Group<W_Group>.
+/// Used for -cc1 frontend options. Driver-only options do not map to
+/// CompilerInvocation.
+multiclass BoolWOption<string flag_base, KeyPathAndMacro kpm,
+                       Default default, FlagDef flag1, FlagDef flag2,
+                       BothFlags both = BothFlags<[]>> {
+  defm NAME : BoolOption<"W", flag_base, kpm, default, flag1, flag2, both>,
+              Group<W_Group>;
+}
+
 // Works like BoolOption except without marshalling
 multiclass BoolOptionWithoutMarshalling<string prefix = "", string spelling_base,
                                         FlagDef flag1_base, FlagDef flag2_base,
@@ -606,6 +617,7 @@ defvar cpp11 = LangOpts<"CPlusPlus11">;
 defvar cpp14 = LangOpts<"CPlusPlus14">;
 defvar cpp17 = LangOpts<"CPlusPlus17">;
 defvar cpp20 = LangOpts<"CPlusPlus20">;
+defvar cpp23 = LangOpts<"CPlusPlus23">;
 defvar c99 = LangOpts<"C99">;
 defvar c23 = LangOpts<"C23">;
 defvar lang_std = LangOpts<"LangStd">;
@@ -961,6 +973,12 @@ def Wdeprecated : Flag<["-"], "Wdeprecated">, Group<W_Group>,
   HelpText<"Enable warnings for deprecated constructs and define __DEPRECATED">;
 def Wno_deprecated : Flag<["-"], "Wno-deprecated">, Group<W_Group>,
   Visibility<[ClangOption, CC1Option]>;
+defm invalid_constexpr : BoolWOption<"invalid-constexpr",
+  LangOpts<"CheckConstexprFunctionBodies">,
+  Default<!strconcat("!", cpp23.KeyPath)>,
+  NegFlag<SetFalse, [], [ClangOption, CC1Option], "Disable">,
+  PosFlag<SetTrue, [], [ClangOption, CC1Option], "Enable">,
+  BothFlags<[], [ClangOption, CC1Option], " checking of constexpr function bodies for validity within a constant expression context">>;
 def Wl_COMMA : CommaJoined<["-"], "Wl,">, Visibility<[ClangOption, FlangOption]>,
   Flags<[LinkerInput, RenderAsInput]>,
   HelpText<"Pass the comma separated arguments in <arg> to the linker">,

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 14ee02c4cd582..58694e5399d58 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2407,6 +2407,9 @@ void CompilerInvocationBase::GenerateDiagnosticArgs(
     // This option is automatically generated from UndefPrefixes.
     if (Warning == "undef-prefix")
       continue;
+    // This option is automatically generated from CheckConstexprFunctionBodies.
+    if (Warning == "invalid-constexpr" || Warning == "no-invalid-constexpr")
+      continue;
     Consumer(StringRef("-W") + Warning);
   }
 

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 631fd4e354927..3c28da1b077cd 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2469,11 +2469,18 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
   //     base class sub-objects shall be a constexpr constructor.
   //
   // Note that this rule is distinct from the "requirements for a constexpr
-  // function", so is not checked in CheckValid mode.
+  // function", so is not checked in CheckValid mode. Because the check for
+  // constexpr potential is expensive, skip the check if the diagnostic is
+  // disabled, the function is declared in a system header, or we're in C++23
+  // or later mode (see https://wg21.link/P2448).
+  bool SkipCheck =
+      !SemaRef.getLangOpts().CheckConstexprFunctionBodies ||
+      SemaRef.getSourceManager().isInSystemHeader(Dcl->getLocation()) ||
+      SemaRef.getDiagnostics().isIgnored(
+          diag::ext_constexpr_function_never_constant_expr, Dcl->getLocation());
   SmallVector<PartialDiagnosticAt, 8> Diags;
-  if (Kind == Sema::CheckConstexprKind::Diagnose &&
-      !Expr::isPotentialConstantExpr(Dcl, Diags) &&
-      !SemaRef.getLangOpts().CPlusPlus23) {
+  if (Kind == Sema::CheckConstexprKind::Diagnose && !SkipCheck &&
+      !Expr::isPotentialConstantExpr(Dcl, Diags)) {
     SemaRef.Diag(Dcl->getLocation(),
                  diag::ext_constexpr_function_never_constant_expr)
         << isa<CXXConstructorDecl>(Dcl) << Dcl->isConsteval()

diff  --git a/clang/test/SemaCXX/constexpr-never-constant.cpp b/clang/test/SemaCXX/constexpr-never-constant.cpp
new file mode 100644
index 0000000000000..307810ee263dd
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-never-constant.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify -fcxx-exceptions %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -Winvalid-constexpr -verify -fcxx-exceptions %s
+// Note: for a diagnostic that defaults to an error, -Wno-foo -Wfoo will
+// disable the diagnostic and then re-enable it *as a warning* rather than as
+// an error. So we manually enable it as an error again with -Werror to keep
+// the diagnostic checks consistent.
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -Wno-invalid-constexpr -Winvalid-constexpr -Werror=invalid-constexpr -verify -fcxx-exceptions %s
+
+// RUN: %clang_cc1 -fsyntax-only -Wno-invalid-constexpr -verify=good -fcxx-exceptions %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -verify=good -fcxx-exceptions %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -Wno-invalid-constexpr -verify=good -fcxx-exceptions %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++23 -Winvalid-constexpr -Wno-invalid-constexpr -verify=good -fcxx-exceptions %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-invalid-constexpr -verify=good -fcxx-exceptions %s
+// good-no-diagnostics
+
+constexpr void func() { // expected-error {{constexpr function never produces a constant expression}}
+  throw 12;             // expected-note {{subexpression not valid in a constant expression}}
+}
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Winvalid-constexpr"
+constexpr void other_func() {
+#pragma clang diagnostic pop
+
+  throw 12;
+}