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
