Hi Marek!

On 2023-09-01T20:00:01-0400, Marek Polacek <[email protected]> wrote:
> 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++.

I noticed that for the following variant code of your
'g++.dg/diagnostic/constexpr3.C':

    struct A {
      A *ap = this;
    };
    
    constexpr A *
    foo (A *a)
    {
      return a;
    }
    
    void
    test ()
    {
      A a_;
      constexpr A *a = foo (&a_); // { dg-error "not a constant expression" }
      // { dg-message "pointer to .a_. is not a constant expression|may differ" 
"" { xfail *-*-* } .-1 }
    }

..., GCC still only diagnoses:

    constexpr3_.C:18:25: error: '& a_' is not a constant expression

..., but doesn't provide any further details (XFAILed 'dg-message').

Compare to Clang:

    constexpr3_.C:18:16: error: constexpr variable 'a' must be initialized by a 
constant expression
       18 |   constexpr A *a = foo (&a_);
          |                ^   ~~~~~~~~~
    constexpr3_.C:18:16: note: pointer to 'a_' is not a constant expression
    constexpr3_.C:17:5: note: declared here
       17 |   A a_;
          |     ^

Is (a) my reasoning/expectation correct, and (b) this worth addressing?


Grüße
 Thomas


> 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 %<static%> "
> +                 "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 00000000000..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

Reply via email to