Re: [PATCH] c++: improve verify_constant diagnostic [PR91483]

2023-09-05 Thread Jason Merrill via Gcc-patches

On 9/1/23 20:00, Marek Polacek wrote:

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

-- >8 --

When verify_constant complains, it's pretty terse.  Consider

   void test ()
   {
 constexpr int i = 42;
 constexpr const int *p = &i;
   }

where it says "'& i' is not a constant expression".  OK, but why?

With this patch, we say:

b.C:5:28: error: '& i' is not a constant expression
 5 |   constexpr const int *p = &i;
   |^~
b.C:5:28: note: pointer to 'i' is not a constant expression
b.C:4:17: note: address of non-static constexpr variable 'i' may differ on each 
invocation of the enclosing function; add 'static' to give it a constant address
 4 |   constexpr int i = 42;
   | ^
   | static

which brings g++ on par with clang++.

gcc/cp/ChangeLog:

* constexpr.cc (verify_constant_explain_r): New.
(verify_constant): Call it.

gcc/testsuite/ChangeLog:

* g++.dg/diagnostic/constexpr3.C: New test.
---
  gcc/cp/constexpr.cc  | 56 +++-
  gcc/testsuite/g++.dg/diagnostic/constexpr3.C | 32 +++
  2 files changed, 87 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/diagnostic/constexpr3.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 8bd5c4a47f8..6d5aed82377 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3381,6 +3381,54 @@ ok:
  }
  }
  
+/* *TP was not deemed constant by reduced_constant_expression_p.  Explain

+   why and suggest what could be done about it.  */
+
+static tree
+verify_constant_explain_r (tree *tp, int *, void *)
+{
+  bool ref_p = false;


I think you'll want something along the lines of

  /* No need to look into types or unevaluated operands.  */
  if (TYPE_P (init) || unevaluated_p (code))
{
  *walk_subtrees = false;
  return NULL_TREE;
}

(from find_uninit_fields_r).

OK with that change.

Jason



Re: [PATCH] c++: improve verify_constant diagnostic [PR91483]

2023-09-01 Thread Marek Polacek via Gcc-patches
On Fri, Sep 01, 2023 at 08:00:01PM -0400, Marek Polacek via Gcc-patches wrote:
>if (TREE_OVERFLOW_P (t))
> diff --git a/gcc/testsuite/g++.dg/diagnostic/constexpr3.C 
> b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
> new file mode 100644
> index 000..b6e43a93664
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
> @@ -0,0 +1,32 @@
> +// { dg-do compile { target c++14 } }

I've added the missing PR c++/91483 here and in the ChangeLog in my local repo.

Marek



[PATCH] c++: improve verify_constant diagnostic [PR91483]

2023-09-01 Thread Marek Polacek via Gcc-patches
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --

When verify_constant complains, it's pretty terse.  Consider

  void test ()
  {
constexpr int i = 42;
constexpr const int *p = &i;
  }

where it says "'& i' is not a constant expression".  OK, but why?

With this patch, we say:

b.C:5:28: error: '& i' is not a constant expression
5 |   constexpr const int *p = &i;
  |^~
b.C:5:28: note: pointer to 'i' is not a constant expression
b.C:4:17: note: address of non-static constexpr variable 'i' may differ on each 
invocation of the enclosing function; add 'static' to give it a constant address
4 |   constexpr int i = 42;
  | ^
  | static

which brings g++ on par with clang++.

gcc/cp/ChangeLog:

* constexpr.cc (verify_constant_explain_r): New.
(verify_constant): Call it.

gcc/testsuite/ChangeLog:

* g++.dg/diagnostic/constexpr3.C: New test.
---
 gcc/cp/constexpr.cc  | 56 +++-
 gcc/testsuite/g++.dg/diagnostic/constexpr3.C | 32 +++
 2 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/constexpr3.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 8bd5c4a47f8..6d5aed82377 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -3381,6 +3381,54 @@ ok:
 }
 }
 
+/* *TP was not deemed constant by reduced_constant_expression_p.  Explain
+   why and suggest what could be done about it.  */
+
+static tree
+verify_constant_explain_r (tree *tp, int *, void *)
+{
+  bool ref_p = false;
+
+  switch (TREE_CODE (*tp))
+{
+CASE_CONVERT:
+  if (TREE_CODE (TREE_OPERAND (*tp, 0)) != ADDR_EXPR)
+   break;
+  ref_p = TYPE_REF_P (TREE_TYPE (*tp));
+  *tp = TREE_OPERAND (*tp, 0);
+  gcc_fallthrough ();
+case ADDR_EXPR:
+  {
+   tree op = TREE_OPERAND (*tp, 0);
+   if (VAR_P (op)
+   && DECL_DECLARED_CONSTEXPR_P (op)
+   && !TREE_STATIC (op)
+   /* ??? We should also say something about temporaries.  */
+   && !DECL_ARTIFICIAL (op))
+ {
+   if (ref_p)
+ inform (location_of (*tp), "reference to %qD is not a constant "
+ "expression", op);
+   else
+ inform (location_of (*tp), "pointer to %qD is not a constant "
+ "expression", op);
+   const location_t op_loc = DECL_SOURCE_LOCATION (op);
+   rich_location richloc (line_table, op_loc);
+   richloc.add_fixit_insert_before (op_loc, "static ");
+   inform (&richloc,
+   "address of non-static constexpr variable %qD may differ on 
"
+   "each invocation of the enclosing function; add % "
+   "to give it a constant address", op);
+ }
+   break;
+  }
+default:
+  break;
+}
+
+  return NULL_TREE;
+}
+
 /* Some expressions may have constant operands but are not constant
themselves, such as 1/0.  Call this function to check for that
condition.
@@ -3398,7 +3446,13 @@ verify_constant (tree t, bool allow_non_constant, bool 
*non_constant_p,
   && t != void_node)
 {
   if (!allow_non_constant)
-   error ("%q+E is not a constant expression", t);
+   {
+ auto_diagnostic_group d;
+ error_at (cp_expr_loc_or_input_loc (t),
+   "%q+E is not a constant expression", t);
+ cp_walk_tree_without_duplicates (&t, verify_constant_explain_r,
+  nullptr);
+   }
   *non_constant_p = true;
 }
   if (TREE_OVERFLOW_P (t))
diff --git a/gcc/testsuite/g++.dg/diagnostic/constexpr3.C 
b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
new file mode 100644
index 000..b6e43a93664
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/constexpr3.C
@@ -0,0 +1,32 @@
+// { dg-do compile { target c++14 } }
+
+struct X {
+  int const& var;
+};
+
+struct A {
+  A *ap = this;
+};
+
+constexpr A
+foo ()
+{
+  return {};
+}
+
+void
+test ()
+{
+  constexpr int i = 42; // { dg-message "may differ on each invocation" }
+
+  constexpr X x{i}; // { dg-error "not a constant expression" }
+  // { dg-message "reference to .i. is not a constant expression" "" { target 
*-*-* } .-1 }
+  constexpr const int *p = &i; // { dg-error "not a constant expression" }
+  // { dg-message "pointer to .i. is not a constant expression" "" { target 
*-*-* } .-1 }
+
+  constexpr A a = foo (); // { dg-error "not a constant expression" }
+  // { dg-message "pointer to .a. is not a constant expression|may differ" "" 
{ target *-*-* } .-1 }
+
+  constexpr const int *q = __builtin_launder (&i); // { dg-error "not a 
constant expression" }
+  // { dg-message "pointer to .i. is not a constant expression" "" { target 
*-*-* } .-1 }
+}

base-commit: 6f06152541d62ae7c8579b7d7bf552be19e15b05
-- 
2.41.0