diff mbox series

c++: Extend std::is_constant_evaluated in if warning [PR100995]

Message ID 20210610014628.2194915-1-polacek@redhat.com
State New
Headers show
Series c++: Extend std::is_constant_evaluated in if warning [PR100995] | expand

Commit Message

Marek Polacek June 10, 2021, 1:46 a.m. UTC
Jakub pointed me at
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1938r3.html#compiler-warnings>
which shows that our existing warning could be extended to handle more
cases.  This patch implements that.

A minor annoyance was handling macros, in libstdc++ we have

  reference operator[](size_type __pos) {
      __glibcxx_assert(__pos <= size());
      ...
  }

wherein __glibcxx_assert expands to

  if (__builtin_is_constant_evaluated() && !bool(__pos <= size())
    ...

but I'm of a mind to not warn on that.

Possible tweaks: merge the "always true" warnings and say something
about a manifestly evaluated context, and perhaps add an early exit
for TREE_CONSTANT trees because for constexpr if we are going to call
maybe_warn_for_constant_evaluated twice.

Once consteval if makes it in, we should tweak this warning one more
time.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR c++/100995

gcc/cp/ChangeLog:

	* semantics.c (find_std_constant_evaluated_r): New.
	(maybe_warn_for_constant_evaluated): New.
	(finish_if_stmt_cond): Call it.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/is-constant-evaluated9.C: Add dg-warning.
	* g++.dg/cpp2a/is-constant-evaluated12.C: New test.
---
 gcc/cp/semantics.c                            | 85 ++++++++++++++++---
 .../g++.dg/cpp2a/is-constant-evaluated12.C    | 79 +++++++++++++++++
 .../g++.dg/cpp2a/is-constant-evaluated9.C     |  4 +-
 3 files changed, 152 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C


base-commit: 4f625f47b4456e5c05a025fca4d072831e59126c

Comments

Jason Merrill June 10, 2021, 2:27 p.m. UTC | #1
On 6/9/21 9:46 PM, Marek Polacek wrote:
> Jakub pointed me at
> <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1938r3.html#compiler-warnings>
> which shows that our existing warning could be extended to handle more
> cases.  This patch implements that.
> 
> A minor annoyance was handling macros, in libstdc++ we have
> 
>    reference operator[](size_type __pos) {
>        __glibcxx_assert(__pos <= size());
>        ...
>    }
> 
> wherein __glibcxx_assert expands to
> 
>    if (__builtin_is_constant_evaluated() && !bool(__pos <= size())
>      ...
> 
> but I'm of a mind to not warn on that.
> 
> Possible tweaks: merge the "always true" warnings and say something
> about a manifestly evaluated context, and perhaps add an early exit
> for TREE_CONSTANT trees because for constexpr if we are going to call
> maybe_warn_for_constant_evaluated twice.
> 
> Once consteval if makes it in, we should tweak this warning one more
> time.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> 	PR c++/100995
> 
> gcc/cp/ChangeLog:
> 
> 	* semantics.c (find_std_constant_evaluated_r): New.
> 	(maybe_warn_for_constant_evaluated): New.
> 	(finish_if_stmt_cond): Call it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/is-constant-evaluated9.C: Add dg-warning.
> 	* g++.dg/cpp2a/is-constant-evaluated12.C: New test.
> ---
>   gcc/cp/semantics.c                            | 85 ++++++++++++++++---
>   .../g++.dg/cpp2a/is-constant-evaluated12.C    | 79 +++++++++++++++++
>   .../g++.dg/cpp2a/is-constant-evaluated9.C     |  4 +-
>   3 files changed, 152 insertions(+), 16 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
> 
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index f506a239864..07459a357e2 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -927,6 +927,75 @@ is_std_constant_evaluated_p (tree fn)
>     return name && id_equal (name, "is_constant_evaluated");
>   }
>   
> +/* Callback function for maybe_warn_for_constant_evaluated that looks
> +   for calls to std::is_constant_evaluated in TP.  */
> +
> +static tree
> +find_std_constant_evaluated_r (tree *tp, int *walk_subtrees, void *)
> +{
> +  tree t = *tp;
> +
> +  if (TYPE_P (t) || TREE_CONSTANT (t))
> +    {
> +      *walk_subtrees = false;
> +      return NULL_TREE;
> +    }
> +
> +  switch (TREE_CODE (t))
> +    {
> +    case CALL_EXPR:
> +      if (is_std_constant_evaluated_p (t))
> +	return t;
> +      break;
> +    case EXPR_STMT:
> +      /* Don't warn in statement expressions.  */
> +      *walk_subtrees = false;
> +      return NULL_TREE;
> +    default:
> +      break;
> +    }
> +
> +  return NULL_TREE;
> +}
> +
> +/* In certain contexts, std::is_constant_evaluated() is always true (for
> +   instance, in a consteval function or in a constexpr if), or always false
> +   (e.g., in a non-constexpr non-consteval function) so give the user a clue.  */
> +
> +static void
> +maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if)
> +{
> +  if (!warn_tautological_compare)
> +    return;
> +
> +  /* Suppress warning for std::is_constant_evaluated if the conditional
> +     comes from a macro.  */
> +  if (from_macro_expansion_at (EXPR_LOCATION (cond)))
> +    return;
> +
> +  cond = cp_walk_tree_without_duplicates (&cond, find_std_constant_evaluated_r,
> +					  NULL);
> +  if (cond)
> +    {
> +      if (constexpr_if)
> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> +		    "%<std::is_constant_evaluated%> always evaluates to "
> +		    "true in %<if constexpr%>");
> +      else if (!DECL_DECLARED_CONSTEXPR_P (current_function_decl)
> +	       /* C++17 lambda op() is implicitly constexpr but finish_function
> +		  may not have marked it as such.  */
> +	       && !(cxx_dialect >= cxx17
> +		    && LAMBDA_TYPE_P (CP_DECL_CONTEXT (current_function_decl))))

Let's factor a new maybe_constexpr_fn out of var_in_maybe_constexpr_fn.

> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> +		    "%<std::is_constant_evaluated%> always evaluates to "
> +		    "false in a non-%<constexpr%> function");
> +      else if (DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> +		    "%<std::is_constant_evaluated%> always evaluates to "
> +		    "true in a %<consteval%> function");
> +    }
> +}
> +
>   /* Process the COND of an if-statement, which may be given by
>      IF_STMT.  */
>   
> @@ -942,23 +1011,11 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
>   	 converted to bool.  */
>         && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node)
>       {
> -      /* if constexpr (std::is_constant_evaluated()) is always true,
> -	 so give the user a clue.  */
> -      if (warn_tautological_compare)
> -	{
> -	  tree t = cond;
> -	  if (TREE_CODE (t) == CLEANUP_POINT_EXPR)
> -	    t = TREE_OPERAND (t, 0);
> -	  if (TREE_CODE (t) == CALL_EXPR
> -	      && is_std_constant_evaluated_p (t))
> -	    warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> -			"%qs always evaluates to true in %<if constexpr%>",
> -			"std::is_constant_evaluated");
> -	}
> -
> +      maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true);
>         cond = instantiate_non_dependent_expr (cond);
>         cond = cxx_constant_value (cond, NULL_TREE);
>       }

Missing an 'else' here?  In your intro you mention calling the function 
twice for constexpr if, but why is that necessary?

> +  maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false);
>     finish_cond (&IF_COND (if_stmt), cond);
>     add_stmt (if_stmt);
>     THEN_CLAUSE (if_stmt) = push_stmt_list ();
> diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
> new file mode 100644
> index 00000000000..0de6bf74f4a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
> @@ -0,0 +1,79 @@
> +// PR c++/100995
> +// { dg-do compile { target c++20 } }
> +// { dg-options "-Wtautological-compare" }
> +
> +namespace std {
> +  constexpr inline bool
> +  is_constant_evaluated () noexcept
> +  {
> +    return __builtin_is_constant_evaluated ();
> +  }
> +}
> +
> +template <int I = []() constexpr { if (std::is_constant_evaluated()) return 1; return 0; }()>
> +struct X { };
> +
> +template <int I = [](){ if (std::is_constant_evaluated()) return 1; return 0; }()>
> +struct Y { };
> +
> +template <int I = [](){ if constexpr (std::is_constant_evaluated()) return 1; return 0; }()> // { dg-warning "always evaluates to true" }
> +struct Z { };
> +
> +constexpr bool b = true;
> +
> +#define __glibcxx_assert(cond) \
> +  if (__builtin_is_constant_evaluated() && !bool(cond)) \
> +__builtin_unreachable()
> +#define CHECK __builtin_is_constant_evaluated() // { dg-warning "always evaluates to false" }
> +#define CHECK2 __builtin_is_constant_evaluated()
> +
> +int
> +foo ()
> +{
> +  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to false" }
> +    return 1;
> +  __glibcxx_assert(b);
> +  if (CHECK && b)
> +    return 2;
> +  if (CHECK2)
> +    return 3;
> +  return 0;
> +}
> +
> +constexpr int
> +bar ()
> +{
> +  if (std::is_constant_evaluated ())
> +    return 1;
> +  if constexpr (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
> +    return 2;
> +  if constexpr (std::is_constant_evaluated () && b) // { dg-warning "always evaluates to true" }
> +    return 3;
> +  if constexpr (!std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
> +    return 4;
> +  return 0;
> +}
> +
> +consteval int
> +baz ()
> +{
> +  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
> +    return 1;
> +  return 0;
> +}
> +
> +int
> +qux ()
> +{
> +  if (({ static bool a = std::is_constant_evaluated (); a; }))
> +    return 1;
> +  if (({ bool a = std::is_constant_evaluated (); a; }))
> +    return 2;
> +  if (static bool a = std::is_constant_evaluated (); a)
> +    return 3;
> +  if (bool a = std::is_constant_evaluated (); a)
> +    return 4;
> +  if constexpr (constexpr bool a = std::is_constant_evaluated (); a)
> +    return 5;
> +  return 0;
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
> index 0e96a1a3729..5e405e71cc0 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
> @@ -32,7 +32,7 @@ constexpr int
>   foo3(int i)
>   {
>     // I is not a constant expression but we short-circuit it.
> -  if constexpr (__builtin_is_constant_evaluated () || i)
> +  if constexpr (__builtin_is_constant_evaluated () || i) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
>       return 42;
>     else
>       return i;
> @@ -42,7 +42,7 @@ constexpr int
>   foo4(int i)
>   {
>     const int j = 0;
> -  if constexpr (j && __builtin_is_constant_evaluated ())
> +  if constexpr (j && __builtin_is_constant_evaluated ()) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
>       return 42;
>     else
>       return i;
> 
> base-commit: 4f625f47b4456e5c05a025fca4d072831e59126c
>
Marek Polacek June 10, 2021, 11:04 p.m. UTC | #2
On Thu, Jun 10, 2021 at 10:27:44AM -0400, Jason Merrill wrote:
> On 6/9/21 9:46 PM, Marek Polacek wrote:
> > Jakub pointed me at
> > <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1938r3.html#compiler-warnings>
> > which shows that our existing warning could be extended to handle more
> > cases.  This patch implements that.
> > 
> > A minor annoyance was handling macros, in libstdc++ we have
> > 
> >    reference operator[](size_type __pos) {
> >        __glibcxx_assert(__pos <= size());
> >        ...
> >    }
> > 
> > wherein __glibcxx_assert expands to
> > 
> >    if (__builtin_is_constant_evaluated() && !bool(__pos <= size())
> >      ...
> > 
> > but I'm of a mind to not warn on that.
> > 
> > Possible tweaks: merge the "always true" warnings and say something
> > about a manifestly evaluated context, and perhaps add an early exit
> > for TREE_CONSTANT trees because for constexpr if we are going to call
> > maybe_warn_for_constant_evaluated twice.
> > 
> > Once consteval if makes it in, we should tweak this warning one more
> > time.
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > 	PR c++/100995
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* semantics.c (find_std_constant_evaluated_r): New.
> > 	(maybe_warn_for_constant_evaluated): New.
> > 	(finish_if_stmt_cond): Call it.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/is-constant-evaluated9.C: Add dg-warning.
> > 	* g++.dg/cpp2a/is-constant-evaluated12.C: New test.
> > ---
> >   gcc/cp/semantics.c                            | 85 ++++++++++++++++---
> >   .../g++.dg/cpp2a/is-constant-evaluated12.C    | 79 +++++++++++++++++
> >   .../g++.dg/cpp2a/is-constant-evaluated9.C     |  4 +-
> >   3 files changed, 152 insertions(+), 16 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
> > 
> > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> > index f506a239864..07459a357e2 100644
> > --- a/gcc/cp/semantics.c
> > +++ b/gcc/cp/semantics.c
> > @@ -927,6 +927,75 @@ is_std_constant_evaluated_p (tree fn)
> >     return name && id_equal (name, "is_constant_evaluated");
> >   }
> > +/* Callback function for maybe_warn_for_constant_evaluated that looks
> > +   for calls to std::is_constant_evaluated in TP.  */
> > +
> > +static tree
> > +find_std_constant_evaluated_r (tree *tp, int *walk_subtrees, void *)
> > +{
> > +  tree t = *tp;
> > +
> > +  if (TYPE_P (t) || TREE_CONSTANT (t))
> > +    {
> > +      *walk_subtrees = false;
> > +      return NULL_TREE;
> > +    }
> > +
> > +  switch (TREE_CODE (t))
> > +    {
> > +    case CALL_EXPR:
> > +      if (is_std_constant_evaluated_p (t))
> > +	return t;
> > +      break;
> > +    case EXPR_STMT:
> > +      /* Don't warn in statement expressions.  */
> > +      *walk_subtrees = false;
> > +      return NULL_TREE;
> > +    default:
> > +      break;
> > +    }
> > +
> > +  return NULL_TREE;
> > +}
> > +
> > +/* In certain contexts, std::is_constant_evaluated() is always true (for
> > +   instance, in a consteval function or in a constexpr if), or always false
> > +   (e.g., in a non-constexpr non-consteval function) so give the user a clue.  */
> > +
> > +static void
> > +maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if)
> > +{
> > +  if (!warn_tautological_compare)
> > +    return;
> > +
> > +  /* Suppress warning for std::is_constant_evaluated if the conditional
> > +     comes from a macro.  */
> > +  if (from_macro_expansion_at (EXPR_LOCATION (cond)))
> > +    return;
> > +
> > +  cond = cp_walk_tree_without_duplicates (&cond, find_std_constant_evaluated_r,
> > +					  NULL);
> > +  if (cond)
> > +    {
> > +      if (constexpr_if)
> > +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> > +		    "%<std::is_constant_evaluated%> always evaluates to "
> > +		    "true in %<if constexpr%>");
> > +      else if (!DECL_DECLARED_CONSTEXPR_P (current_function_decl)
> > +	       /* C++17 lambda op() is implicitly constexpr but finish_function
> > +		  may not have marked it as such.  */
> > +	       && !(cxx_dialect >= cxx17
> > +		    && LAMBDA_TYPE_P (CP_DECL_CONTEXT (current_function_decl))))
> 
> Let's factor a new maybe_constexpr_fn out of var_in_maybe_constexpr_fn.

I've introduced maybe_constexpr_fn but I don't really see a way to
use it in var_in_maybe_constexpr_fn that would simplify things and
didn't change its semantics.
 
> > +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> > +		    "%<std::is_constant_evaluated%> always evaluates to "
> > +		    "false in a non-%<constexpr%> function");
> > +      else if (DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
> > +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> > +		    "%<std::is_constant_evaluated%> always evaluates to "
> > +		    "true in a %<consteval%> function");
> > +    }
> > +}
> > +
> >   /* Process the COND of an if-statement, which may be given by
> >      IF_STMT.  */
> > @@ -942,23 +1011,11 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
> >   	 converted to bool.  */
> >         && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node)
> >       {
> > -      /* if constexpr (std::is_constant_evaluated()) is always true,
> > -	 so give the user a clue.  */
> > -      if (warn_tautological_compare)
> > -	{
> > -	  tree t = cond;
> > -	  if (TREE_CODE (t) == CLEANUP_POINT_EXPR)
> > -	    t = TREE_OPERAND (t, 0);
> > -	  if (TREE_CODE (t) == CALL_EXPR
> > -	      && is_std_constant_evaluated_p (t))
> > -	    warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> > -			"%qs always evaluates to true in %<if constexpr%>",
> > -			"std::is_constant_evaluated");
> > -	}
> > -
> > +      maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true);
> >         cond = instantiate_non_dependent_expr (cond);
> >         cond = cxx_constant_value (cond, NULL_TREE);
> >       }
> 
> Missing an 'else' here?  In your intro you mention calling the function
> twice for constexpr if, but why is that necessary?

Oof, that's some weird thinko.  I've added the missing else, thanks.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Jakub pointed me at
<http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1938r3.html#compiler-warnings>
which shows that our existing warning could be extended to handle more
cases.  This patch implements that.

A minor annoyance was handling macros, in libstdc++ we have

  reference operator[](size_type __pos) {
      __glibcxx_assert(__pos <= size());
      ...
  }

wherein __glibcxx_assert expands to

  if (__builtin_is_constant_evaluated() && !bool(__pos <= size())
    ...

but I'm of a mind to not warn on that.

Once consteval if makes it in, we should tweak this warning one more
time.

	PR c++/100995

gcc/cp/ChangeLog:

	* constexpr.c (maybe_constexpr_fn): New.
	* cp-tree.h (maybe_constexpr_fn): Declare.
	* semantics.c (find_std_constant_evaluated_r): New.
	(maybe_warn_for_constant_evaluated): New.
	(finish_if_stmt_cond): Call it.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/is-constant-evaluated9.C: Add dg-warning.
	* g++.dg/cpp2a/is-constant-evaluated12.C: New test.
---
 gcc/cp/constexpr.c                            | 10 +++
 gcc/cp/cp-tree.h                              |  1 +
 gcc/cp/semantics.c                            | 82 +++++++++++++++----
 .../g++.dg/cpp2a/is-constant-evaluated12.C    | 79 ++++++++++++++++++
 .../g++.dg/cpp2a/is-constant-evaluated9.C     |  4 +-
 5 files changed, 160 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 297f2072de8..01b0c42471d 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5121,6 +5121,16 @@ var_in_constexpr_fn (tree t)
 	  && DECL_DECLARED_CONSTEXPR_P (ctx));
 }
 
+/* True if a function might be constexpr: either a function that was
+   declared constexpr, or a C++17 lambda op().  */
+
+bool
+maybe_constexpr_fn (tree t)
+{
+  return (DECL_DECLARED_CONSTEXPR_P (t)
+	  || (cxx_dialect >= cxx17 && LAMBDA_FUNCTION_P (t)));
+}
+
 /* True if T was declared in a function that might be constexpr: either a
    function that was declared constexpr, or a C++17 lambda op().  */
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b1b7e615bcc..9ac8b527749 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8244,6 +8244,7 @@ extern bool reduced_constant_expression_p       (tree);
 extern bool is_instantiation_of_constexpr       (tree);
 extern bool var_in_constexpr_fn                 (tree);
 extern bool var_in_maybe_constexpr_fn           (tree);
+extern bool maybe_constexpr_fn			(tree);
 extern void explain_invalid_constexpr_fn        (tree);
 extern vec<tree> cx_error_context               (void);
 extern tree fold_sizeof_expr			(tree);
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index f506a239864..384c54bd025 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -927,6 +927,71 @@ is_std_constant_evaluated_p (tree fn)
   return name && id_equal (name, "is_constant_evaluated");
 }
 
+/* Callback function for maybe_warn_for_constant_evaluated that looks
+   for calls to std::is_constant_evaluated in TP.  */
+
+static tree
+find_std_constant_evaluated_r (tree *tp, int *walk_subtrees, void *)
+{
+  tree t = *tp;
+
+  if (TYPE_P (t) || TREE_CONSTANT (t))
+    {
+      *walk_subtrees = false;
+      return NULL_TREE;
+    }
+
+  switch (TREE_CODE (t))
+    {
+    case CALL_EXPR:
+      if (is_std_constant_evaluated_p (t))
+	return t;
+      break;
+    case EXPR_STMT:
+      /* Don't warn in statement expressions.  */
+      *walk_subtrees = false;
+      return NULL_TREE;
+    default:
+      break;
+    }
+
+  return NULL_TREE;
+}
+
+/* In certain contexts, std::is_constant_evaluated() is always true (for
+   instance, in a consteval function or in a constexpr if), or always false
+   (e.g., in a non-constexpr non-consteval function) so give the user a clue.  */
+
+static void
+maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if)
+{
+  if (!warn_tautological_compare)
+    return;
+
+  /* Suppress warning for std::is_constant_evaluated if the conditional
+     comes from a macro.  */
+  if (from_macro_expansion_at (EXPR_LOCATION (cond)))
+    return;
+
+  cond = cp_walk_tree_without_duplicates (&cond, find_std_constant_evaluated_r,
+					  NULL);
+  if (cond)
+    {
+      if (constexpr_if)
+	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
+		    "%<std::is_constant_evaluated%> always evaluates to "
+		    "true in %<if constexpr%>");
+      else if (!maybe_constexpr_fn (current_function_decl))
+	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
+		    "%<std::is_constant_evaluated%> always evaluates to "
+		    "false in a non-%<constexpr%> function");
+      else if (DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
+	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
+		    "%<std::is_constant_evaluated%> always evaluates to "
+		    "true in a %<consteval%> function");
+    }
+}
+
 /* Process the COND of an if-statement, which may be given by
    IF_STMT.  */
 
@@ -942,23 +1007,12 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
 	 converted to bool.  */
       && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node)
     {
-      /* if constexpr (std::is_constant_evaluated()) is always true,
-	 so give the user a clue.  */
-      if (warn_tautological_compare)
-	{
-	  tree t = cond;
-	  if (TREE_CODE (t) == CLEANUP_POINT_EXPR)
-	    t = TREE_OPERAND (t, 0);
-	  if (TREE_CODE (t) == CALL_EXPR
-	      && is_std_constant_evaluated_p (t))
-	    warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
-			"%qs always evaluates to true in %<if constexpr%>",
-			"std::is_constant_evaluated");
-	}
-
+      maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true);
       cond = instantiate_non_dependent_expr (cond);
       cond = cxx_constant_value (cond, NULL_TREE);
     }
+  else
+    maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false);
   finish_cond (&IF_COND (if_stmt), cond);
   add_stmt (if_stmt);
   THEN_CLAUSE (if_stmt) = push_stmt_list ();
diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
new file mode 100644
index 00000000000..0de6bf74f4a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
@@ -0,0 +1,79 @@
+// PR c++/100995
+// { dg-do compile { target c++20 } }
+// { dg-options "-Wtautological-compare" }
+
+namespace std {
+  constexpr inline bool
+  is_constant_evaluated () noexcept
+  {
+    return __builtin_is_constant_evaluated ();
+  }
+}
+
+template <int I = []() constexpr { if (std::is_constant_evaluated()) return 1; return 0; }()>
+struct X { };
+
+template <int I = [](){ if (std::is_constant_evaluated()) return 1; return 0; }()>
+struct Y { };
+
+template <int I = [](){ if constexpr (std::is_constant_evaluated()) return 1; return 0; }()> // { dg-warning "always evaluates to true" }
+struct Z { };
+
+constexpr bool b = true;
+
+#define __glibcxx_assert(cond) \
+  if (__builtin_is_constant_evaluated() && !bool(cond)) \
+__builtin_unreachable()
+#define CHECK __builtin_is_constant_evaluated() // { dg-warning "always evaluates to false" }
+#define CHECK2 __builtin_is_constant_evaluated()
+
+int
+foo ()
+{
+  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to false" }
+    return 1;
+  __glibcxx_assert(b);
+  if (CHECK && b)
+    return 2;
+  if (CHECK2)
+    return 3;
+  return 0;
+}
+
+constexpr int
+bar ()
+{
+  if (std::is_constant_evaluated ())
+    return 1;
+  if constexpr (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
+    return 2;
+  if constexpr (std::is_constant_evaluated () && b) // { dg-warning "always evaluates to true" }
+    return 3;
+  if constexpr (!std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
+    return 4;
+  return 0;
+}
+
+consteval int
+baz ()
+{
+  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
+    return 1;
+  return 0;
+}
+
+int
+qux ()
+{
+  if (({ static bool a = std::is_constant_evaluated (); a; }))
+    return 1;
+  if (({ bool a = std::is_constant_evaluated (); a; }))
+    return 2;
+  if (static bool a = std::is_constant_evaluated (); a)
+    return 3;
+  if (bool a = std::is_constant_evaluated (); a)
+    return 4;
+  if constexpr (constexpr bool a = std::is_constant_evaluated (); a)
+    return 5;
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
index 0e96a1a3729..5e405e71cc0 100644
--- a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
+++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
@@ -32,7 +32,7 @@ constexpr int
 foo3(int i)
 {
   // I is not a constant expression but we short-circuit it.
-  if constexpr (__builtin_is_constant_evaluated () || i)
+  if constexpr (__builtin_is_constant_evaluated () || i) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
     return 42;
   else
     return i;
@@ -42,7 +42,7 @@ constexpr int
 foo4(int i)
 {
   const int j = 0;
-  if constexpr (j && __builtin_is_constant_evaluated ())
+  if constexpr (j && __builtin_is_constant_evaluated ()) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
     return 42;
   else
     return i;

base-commit: 7a895955095b6f4d9fcf3b6686dc1113591da28d
Jason Merrill June 11, 2021, 2:25 a.m. UTC | #3
On 6/10/21 7:04 PM, Marek Polacek wrote:
> On Thu, Jun 10, 2021 at 10:27:44AM -0400, Jason Merrill wrote:
>> On 6/9/21 9:46 PM, Marek Polacek wrote:
>>> Jakub pointed me at
>>> <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1938r3.html#compiler-warnings>
>>> which shows that our existing warning could be extended to handle more
>>> cases.  This patch implements that.
>>>
>>> A minor annoyance was handling macros, in libstdc++ we have
>>>
>>>     reference operator[](size_type __pos) {
>>>         __glibcxx_assert(__pos <= size());
>>>         ...
>>>     }
>>>
>>> wherein __glibcxx_assert expands to
>>>
>>>     if (__builtin_is_constant_evaluated() && !bool(__pos <= size())
>>>       ...
>>>
>>> but I'm of a mind to not warn on that.
>>>
>>> Possible tweaks: merge the "always true" warnings and say something
>>> about a manifestly evaluated context, and perhaps add an early exit
>>> for TREE_CONSTANT trees because for constexpr if we are going to call
>>> maybe_warn_for_constant_evaluated twice.
>>>
>>> Once consteval if makes it in, we should tweak this warning one more
>>> time.
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> 	PR c++/100995
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* semantics.c (find_std_constant_evaluated_r): New.
>>> 	(maybe_warn_for_constant_evaluated): New.
>>> 	(finish_if_stmt_cond): Call it.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp2a/is-constant-evaluated9.C: Add dg-warning.
>>> 	* g++.dg/cpp2a/is-constant-evaluated12.C: New test.
>>> ---
>>>    gcc/cp/semantics.c                            | 85 ++++++++++++++++---
>>>    .../g++.dg/cpp2a/is-constant-evaluated12.C    | 79 +++++++++++++++++
>>>    .../g++.dg/cpp2a/is-constant-evaluated9.C     |  4 +-
>>>    3 files changed, 152 insertions(+), 16 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
>>>
>>> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
>>> index f506a239864..07459a357e2 100644
>>> --- a/gcc/cp/semantics.c
>>> +++ b/gcc/cp/semantics.c
>>> @@ -927,6 +927,75 @@ is_std_constant_evaluated_p (tree fn)
>>>      return name && id_equal (name, "is_constant_evaluated");
>>>    }
>>> +/* Callback function for maybe_warn_for_constant_evaluated that looks
>>> +   for calls to std::is_constant_evaluated in TP.  */
>>> +
>>> +static tree
>>> +find_std_constant_evaluated_r (tree *tp, int *walk_subtrees, void *)
>>> +{
>>> +  tree t = *tp;
>>> +
>>> +  if (TYPE_P (t) || TREE_CONSTANT (t))
>>> +    {
>>> +      *walk_subtrees = false;
>>> +      return NULL_TREE;
>>> +    }
>>> +
>>> +  switch (TREE_CODE (t))
>>> +    {
>>> +    case CALL_EXPR:
>>> +      if (is_std_constant_evaluated_p (t))
>>> +	return t;
>>> +      break;
>>> +    case EXPR_STMT:
>>> +      /* Don't warn in statement expressions.  */
>>> +      *walk_subtrees = false;
>>> +      return NULL_TREE;
>>> +    default:
>>> +      break;
>>> +    }
>>> +
>>> +  return NULL_TREE;
>>> +}
>>> +
>>> +/* In certain contexts, std::is_constant_evaluated() is always true (for
>>> +   instance, in a consteval function or in a constexpr if), or always false
>>> +   (e.g., in a non-constexpr non-consteval function) so give the user a clue.  */
>>> +
>>> +static void
>>> +maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if)
>>> +{
>>> +  if (!warn_tautological_compare)
>>> +    return;
>>> +
>>> +  /* Suppress warning for std::is_constant_evaluated if the conditional
>>> +     comes from a macro.  */
>>> +  if (from_macro_expansion_at (EXPR_LOCATION (cond)))
>>> +    return;
>>> +
>>> +  cond = cp_walk_tree_without_duplicates (&cond, find_std_constant_evaluated_r,
>>> +					  NULL);
>>> +  if (cond)
>>> +    {
>>> +      if (constexpr_if)
>>> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
>>> +		    "%<std::is_constant_evaluated%> always evaluates to "
>>> +		    "true in %<if constexpr%>");
>>> +      else if (!DECL_DECLARED_CONSTEXPR_P (current_function_decl)
>>> +	       /* C++17 lambda op() is implicitly constexpr but finish_function
>>> +		  may not have marked it as such.  */
>>> +	       && !(cxx_dialect >= cxx17
>>> +		    && LAMBDA_TYPE_P (CP_DECL_CONTEXT (current_function_decl))))
>>
>> Let's factor a new maybe_constexpr_fn out of var_in_maybe_constexpr_fn.
> 
> I've introduced maybe_constexpr_fn but I don't really see a way to
> use it in var_in_maybe_constexpr_fn that would simplify things and
> didn't change its semantics.
>   
>>> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
>>> +		    "%<std::is_constant_evaluated%> always evaluates to "
>>> +		    "false in a non-%<constexpr%> function");
>>> +      else if (DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
>>> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
>>> +		    "%<std::is_constant_evaluated%> always evaluates to "
>>> +		    "true in a %<consteval%> function");
>>> +    }
>>> +}
>>> +
>>>    /* Process the COND of an if-statement, which may be given by
>>>       IF_STMT.  */
>>> @@ -942,23 +1011,11 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
>>>    	 converted to bool.  */
>>>          && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node)
>>>        {
>>> -      /* if constexpr (std::is_constant_evaluated()) is always true,
>>> -	 so give the user a clue.  */
>>> -      if (warn_tautological_compare)
>>> -	{
>>> -	  tree t = cond;
>>> -	  if (TREE_CODE (t) == CLEANUP_POINT_EXPR)
>>> -	    t = TREE_OPERAND (t, 0);
>>> -	  if (TREE_CODE (t) == CALL_EXPR
>>> -	      && is_std_constant_evaluated_p (t))
>>> -	    warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
>>> -			"%qs always evaluates to true in %<if constexpr%>",
>>> -			"std::is_constant_evaluated");
>>> -	}
>>> -
>>> +      maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true);
>>>          cond = instantiate_non_dependent_expr (cond);
>>>          cond = cxx_constant_value (cond, NULL_TREE);
>>>        }
>>
>> Missing an 'else' here?  In your intro you mention calling the function
>> twice for constexpr if, but why is that necessary?
> 
> Oof, that's some weird thinko.  I've added the missing else, thanks.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

> -- >8 --
> Jakub pointed me at
> <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1938r3.html#compiler-warnings>
> which shows that our existing warning could be extended to handle more
> cases.  This patch implements that.
> 
> A minor annoyance was handling macros, in libstdc++ we have
> 
>    reference operator[](size_type __pos) {
>        __glibcxx_assert(__pos <= size());
>        ...
>    }
> 
> wherein __glibcxx_assert expands to
> 
>    if (__builtin_is_constant_evaluated() && !bool(__pos <= size())
>      ...
> 
> but I'm of a mind to not warn on that.
> 
> Once consteval if makes it in, we should tweak this warning one more
> time.
> 
> 	PR c++/100995
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.c (maybe_constexpr_fn): New.
> 	* cp-tree.h (maybe_constexpr_fn): Declare.
> 	* semantics.c (find_std_constant_evaluated_r): New.
> 	(maybe_warn_for_constant_evaluated): New.
> 	(finish_if_stmt_cond): Call it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/is-constant-evaluated9.C: Add dg-warning.
> 	* g++.dg/cpp2a/is-constant-evaluated12.C: New test.
> ---
>   gcc/cp/constexpr.c                            | 10 +++
>   gcc/cp/cp-tree.h                              |  1 +
>   gcc/cp/semantics.c                            | 82 +++++++++++++++----
>   .../g++.dg/cpp2a/is-constant-evaluated12.C    | 79 ++++++++++++++++++
>   .../g++.dg/cpp2a/is-constant-evaluated9.C     |  4 +-
>   5 files changed, 160 insertions(+), 16 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 297f2072de8..01b0c42471d 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -5121,6 +5121,16 @@ var_in_constexpr_fn (tree t)
>   	  && DECL_DECLARED_CONSTEXPR_P (ctx));
>   }
>   
> +/* True if a function might be constexpr: either a function that was
> +   declared constexpr, or a C++17 lambda op().  */
> +
> +bool
> +maybe_constexpr_fn (tree t)
> +{
> +  return (DECL_DECLARED_CONSTEXPR_P (t)
> +	  || (cxx_dialect >= cxx17 && LAMBDA_FUNCTION_P (t)));
> +}
> +
>   /* True if T was declared in a function that might be constexpr: either a
>      function that was declared constexpr, or a C++17 lambda op().  */
>   
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index b1b7e615bcc..9ac8b527749 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -8244,6 +8244,7 @@ extern bool reduced_constant_expression_p       (tree);
>   extern bool is_instantiation_of_constexpr       (tree);
>   extern bool var_in_constexpr_fn                 (tree);
>   extern bool var_in_maybe_constexpr_fn           (tree);
> +extern bool maybe_constexpr_fn			(tree);
>   extern void explain_invalid_constexpr_fn        (tree);
>   extern vec<tree> cx_error_context               (void);
>   extern tree fold_sizeof_expr			(tree);
> diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> index f506a239864..384c54bd025 100644
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -927,6 +927,71 @@ is_std_constant_evaluated_p (tree fn)
>     return name && id_equal (name, "is_constant_evaluated");
>   }
>   
> +/* Callback function for maybe_warn_for_constant_evaluated that looks
> +   for calls to std::is_constant_evaluated in TP.  */
> +
> +static tree
> +find_std_constant_evaluated_r (tree *tp, int *walk_subtrees, void *)
> +{
> +  tree t = *tp;
> +
> +  if (TYPE_P (t) || TREE_CONSTANT (t))
> +    {
> +      *walk_subtrees = false;
> +      return NULL_TREE;
> +    }
> +
> +  switch (TREE_CODE (t))
> +    {
> +    case CALL_EXPR:
> +      if (is_std_constant_evaluated_p (t))
> +	return t;
> +      break;
> +    case EXPR_STMT:
> +      /* Don't warn in statement expressions.  */
> +      *walk_subtrees = false;
> +      return NULL_TREE;
> +    default:
> +      break;
> +    }
> +
> +  return NULL_TREE;
> +}
> +
> +/* In certain contexts, std::is_constant_evaluated() is always true (for
> +   instance, in a consteval function or in a constexpr if), or always false
> +   (e.g., in a non-constexpr non-consteval function) so give the user a clue.  */
> +
> +static void
> +maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if)
> +{
> +  if (!warn_tautological_compare)
> +    return;
> +
> +  /* Suppress warning for std::is_constant_evaluated if the conditional
> +     comes from a macro.  */
> +  if (from_macro_expansion_at (EXPR_LOCATION (cond)))
> +    return;
> +
> +  cond = cp_walk_tree_without_duplicates (&cond, find_std_constant_evaluated_r,
> +					  NULL);
> +  if (cond)
> +    {
> +      if (constexpr_if)
> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> +		    "%<std::is_constant_evaluated%> always evaluates to "
> +		    "true in %<if constexpr%>");
> +      else if (!maybe_constexpr_fn (current_function_decl))
> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> +		    "%<std::is_constant_evaluated%> always evaluates to "
> +		    "false in a non-%<constexpr%> function");
> +      else if (DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
> +	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> +		    "%<std::is_constant_evaluated%> always evaluates to "
> +		    "true in a %<consteval%> function");
> +    }
> +}
> +
>   /* Process the COND of an if-statement, which may be given by
>      IF_STMT.  */
>   
> @@ -942,23 +1007,12 @@ finish_if_stmt_cond (tree cond, tree if_stmt)
>   	 converted to bool.  */
>         && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node)
>       {
> -      /* if constexpr (std::is_constant_evaluated()) is always true,
> -	 so give the user a clue.  */
> -      if (warn_tautological_compare)
> -	{
> -	  tree t = cond;
> -	  if (TREE_CODE (t) == CLEANUP_POINT_EXPR)
> -	    t = TREE_OPERAND (t, 0);
> -	  if (TREE_CODE (t) == CALL_EXPR
> -	      && is_std_constant_evaluated_p (t))
> -	    warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
> -			"%qs always evaluates to true in %<if constexpr%>",
> -			"std::is_constant_evaluated");
> -	}
> -
> +      maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true);
>         cond = instantiate_non_dependent_expr (cond);
>         cond = cxx_constant_value (cond, NULL_TREE);
>       }
> +  else
> +    maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false);
>     finish_cond (&IF_COND (if_stmt), cond);
>     add_stmt (if_stmt);
>     THEN_CLAUSE (if_stmt) = push_stmt_list ();
> diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
> new file mode 100644
> index 00000000000..0de6bf74f4a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
> @@ -0,0 +1,79 @@
> +// PR c++/100995
> +// { dg-do compile { target c++20 } }
> +// { dg-options "-Wtautological-compare" }
> +
> +namespace std {
> +  constexpr inline bool
> +  is_constant_evaluated () noexcept
> +  {
> +    return __builtin_is_constant_evaluated ();
> +  }
> +}
> +
> +template <int I = []() constexpr { if (std::is_constant_evaluated()) return 1; return 0; }()>
> +struct X { };
> +
> +template <int I = [](){ if (std::is_constant_evaluated()) return 1; return 0; }()>
> +struct Y { };
> +
> +template <int I = [](){ if constexpr (std::is_constant_evaluated()) return 1; return 0; }()> // { dg-warning "always evaluates to true" }
> +struct Z { };
> +
> +constexpr bool b = true;
> +
> +#define __glibcxx_assert(cond) \
> +  if (__builtin_is_constant_evaluated() && !bool(cond)) \
> +__builtin_unreachable()
> +#define CHECK __builtin_is_constant_evaluated() // { dg-warning "always evaluates to false" }
> +#define CHECK2 __builtin_is_constant_evaluated()
> +
> +int
> +foo ()
> +{
> +  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to false" }
> +    return 1;
> +  __glibcxx_assert(b);
> +  if (CHECK && b)
> +    return 2;
> +  if (CHECK2)
> +    return 3;
> +  return 0;
> +}
> +
> +constexpr int
> +bar ()
> +{
> +  if (std::is_constant_evaluated ())
> +    return 1;
> +  if constexpr (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
> +    return 2;
> +  if constexpr (std::is_constant_evaluated () && b) // { dg-warning "always evaluates to true" }
> +    return 3;
> +  if constexpr (!std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
> +    return 4;
> +  return 0;
> +}
> +
> +consteval int
> +baz ()
> +{
> +  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
> +    return 1;
> +  return 0;
> +}
> +
> +int
> +qux ()
> +{
> +  if (({ static bool a = std::is_constant_evaluated (); a; }))
> +    return 1;
> +  if (({ bool a = std::is_constant_evaluated (); a; }))
> +    return 2;
> +  if (static bool a = std::is_constant_evaluated (); a)
> +    return 3;
> +  if (bool a = std::is_constant_evaluated (); a)
> +    return 4;
> +  if constexpr (constexpr bool a = std::is_constant_evaluated (); a)
> +    return 5;
> +  return 0;
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
> index 0e96a1a3729..5e405e71cc0 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
> @@ -32,7 +32,7 @@ constexpr int
>   foo3(int i)
>   {
>     // I is not a constant expression but we short-circuit it.
> -  if constexpr (__builtin_is_constant_evaluated () || i)
> +  if constexpr (__builtin_is_constant_evaluated () || i) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
>       return 42;
>     else
>       return i;
> @@ -42,7 +42,7 @@ constexpr int
>   foo4(int i)
>   {
>     const int j = 0;
> -  if constexpr (j && __builtin_is_constant_evaluated ())
> +  if constexpr (j && __builtin_is_constant_evaluated ()) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
>       return 42;
>     else
>       return i;
> 
> base-commit: 7a895955095b6f4d9fcf3b6686dc1113591da28d
>
diff mbox series

Patch

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index f506a239864..07459a357e2 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -927,6 +927,75 @@  is_std_constant_evaluated_p (tree fn)
   return name && id_equal (name, "is_constant_evaluated");
 }
 
+/* Callback function for maybe_warn_for_constant_evaluated that looks
+   for calls to std::is_constant_evaluated in TP.  */
+
+static tree
+find_std_constant_evaluated_r (tree *tp, int *walk_subtrees, void *)
+{
+  tree t = *tp;
+
+  if (TYPE_P (t) || TREE_CONSTANT (t))
+    {
+      *walk_subtrees = false;
+      return NULL_TREE;
+    }
+
+  switch (TREE_CODE (t))
+    {
+    case CALL_EXPR:
+      if (is_std_constant_evaluated_p (t))
+	return t;
+      break;
+    case EXPR_STMT:
+      /* Don't warn in statement expressions.  */
+      *walk_subtrees = false;
+      return NULL_TREE;
+    default:
+      break;
+    }
+
+  return NULL_TREE;
+}
+
+/* In certain contexts, std::is_constant_evaluated() is always true (for
+   instance, in a consteval function or in a constexpr if), or always false
+   (e.g., in a non-constexpr non-consteval function) so give the user a clue.  */
+
+static void
+maybe_warn_for_constant_evaluated (tree cond, bool constexpr_if)
+{
+  if (!warn_tautological_compare)
+    return;
+
+  /* Suppress warning for std::is_constant_evaluated if the conditional
+     comes from a macro.  */
+  if (from_macro_expansion_at (EXPR_LOCATION (cond)))
+    return;
+
+  cond = cp_walk_tree_without_duplicates (&cond, find_std_constant_evaluated_r,
+					  NULL);
+  if (cond)
+    {
+      if (constexpr_if)
+	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
+		    "%<std::is_constant_evaluated%> always evaluates to "
+		    "true in %<if constexpr%>");
+      else if (!DECL_DECLARED_CONSTEXPR_P (current_function_decl)
+	       /* C++17 lambda op() is implicitly constexpr but finish_function
+		  may not have marked it as such.  */
+	       && !(cxx_dialect >= cxx17
+		    && LAMBDA_TYPE_P (CP_DECL_CONTEXT (current_function_decl))))
+	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
+		    "%<std::is_constant_evaluated%> always evaluates to "
+		    "false in a non-%<constexpr%> function");
+      else if (DECL_IMMEDIATE_FUNCTION_P (current_function_decl))
+	warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
+		    "%<std::is_constant_evaluated%> always evaluates to "
+		    "true in a %<consteval%> function");
+    }
+}
+
 /* Process the COND of an if-statement, which may be given by
    IF_STMT.  */
 
@@ -942,23 +1011,11 @@  finish_if_stmt_cond (tree cond, tree if_stmt)
 	 converted to bool.  */
       && TYPE_MAIN_VARIANT (TREE_TYPE (cond)) == boolean_type_node)
     {
-      /* if constexpr (std::is_constant_evaluated()) is always true,
-	 so give the user a clue.  */
-      if (warn_tautological_compare)
-	{
-	  tree t = cond;
-	  if (TREE_CODE (t) == CLEANUP_POINT_EXPR)
-	    t = TREE_OPERAND (t, 0);
-	  if (TREE_CODE (t) == CALL_EXPR
-	      && is_std_constant_evaluated_p (t))
-	    warning_at (EXPR_LOCATION (cond), OPT_Wtautological_compare,
-			"%qs always evaluates to true in %<if constexpr%>",
-			"std::is_constant_evaluated");
-	}
-
+      maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/true);
       cond = instantiate_non_dependent_expr (cond);
       cond = cxx_constant_value (cond, NULL_TREE);
     }
+  maybe_warn_for_constant_evaluated (cond, /*constexpr_if=*/false);
   finish_cond (&IF_COND (if_stmt), cond);
   add_stmt (if_stmt);
   THEN_CLAUSE (if_stmt) = push_stmt_list ();
diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
new file mode 100644
index 00000000000..0de6bf74f4a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated12.C
@@ -0,0 +1,79 @@ 
+// PR c++/100995
+// { dg-do compile { target c++20 } }
+// { dg-options "-Wtautological-compare" }
+
+namespace std {
+  constexpr inline bool
+  is_constant_evaluated () noexcept
+  {
+    return __builtin_is_constant_evaluated ();
+  }
+}
+
+template <int I = []() constexpr { if (std::is_constant_evaluated()) return 1; return 0; }()>
+struct X { };
+
+template <int I = [](){ if (std::is_constant_evaluated()) return 1; return 0; }()>
+struct Y { };
+
+template <int I = [](){ if constexpr (std::is_constant_evaluated()) return 1; return 0; }()> // { dg-warning "always evaluates to true" }
+struct Z { };
+
+constexpr bool b = true;
+
+#define __glibcxx_assert(cond) \
+  if (__builtin_is_constant_evaluated() && !bool(cond)) \
+__builtin_unreachable()
+#define CHECK __builtin_is_constant_evaluated() // { dg-warning "always evaluates to false" }
+#define CHECK2 __builtin_is_constant_evaluated()
+
+int
+foo ()
+{
+  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to false" }
+    return 1;
+  __glibcxx_assert(b);
+  if (CHECK && b)
+    return 2;
+  if (CHECK2)
+    return 3;
+  return 0;
+}
+
+constexpr int
+bar ()
+{
+  if (std::is_constant_evaluated ())
+    return 1;
+  if constexpr (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
+    return 2;
+  if constexpr (std::is_constant_evaluated () && b) // { dg-warning "always evaluates to true" }
+    return 3;
+  if constexpr (!std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
+    return 4;
+  return 0;
+}
+
+consteval int
+baz ()
+{
+  if (std::is_constant_evaluated ()) // { dg-warning "always evaluates to true" }
+    return 1;
+  return 0;
+}
+
+int
+qux ()
+{
+  if (({ static bool a = std::is_constant_evaluated (); a; }))
+    return 1;
+  if (({ bool a = std::is_constant_evaluated (); a; }))
+    return 2;
+  if (static bool a = std::is_constant_evaluated (); a)
+    return 3;
+  if (bool a = std::is_constant_evaluated (); a)
+    return 4;
+  if constexpr (constexpr bool a = std::is_constant_evaluated (); a)
+    return 5;
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
index 0e96a1a3729..5e405e71cc0 100644
--- a/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
+++ b/gcc/testsuite/g++.dg/cpp2a/is-constant-evaluated9.C
@@ -32,7 +32,7 @@  constexpr int
 foo3(int i)
 {
   // I is not a constant expression but we short-circuit it.
-  if constexpr (__builtin_is_constant_evaluated () || i)
+  if constexpr (__builtin_is_constant_evaluated () || i) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
     return 42;
   else
     return i;
@@ -42,7 +42,7 @@  constexpr int
 foo4(int i)
 {
   const int j = 0;
-  if constexpr (j && __builtin_is_constant_evaluated ())
+  if constexpr (j && __builtin_is_constant_evaluated ()) // { dg-warning ".std::is_constant_evaluated. always evaluates to true in .if constexpr." }
     return 42;
   else
     return i;