[Bug c++/105050] New: error: expression '<statement>' is not a constant expression

[Bug c++/105050] New: error: expression '<statement>' is not a constant expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

            Bug ID: 105050
           Summary: error: expression '<statement>' is not a constant
                    expression
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---

template<typename _Tp>
  struct expected
  {
    void _M_swap_val(expected&) { }

    void _M_swap_unex(expected&) { }

    void _M_swap_val_unex(expected&) { }

    bool _M_has_value = true;

    constexpr void
    swap(expected& __x)
    {
      if (this->has_value())
      {
        if (__x.has_value())
          this->_M_swap_val(__x);
        else
          this->_M_swap_val_unex(__x);
      }
      else
      {
        if (__x.has_value())
          __x._M_swap_val_unex(*this);
        else
          this->_M_swap_unex(__x);
      }
    }

    constexpr bool has_value() const noexcept { return this->_M_has_value; }
  };

constexpr bool
test_swap()
{
  expected<int> e1, e2;
  e1.swap(e2);

  return true;
}

static_assert(test_swap());



This doesn't give a very helpful diagnostic:

x.ii:43:24: error: non-constant condition for static assertion
   43 | static_assert(test_swap());
      |               ~~~~~~~~~^~
x.ii:43:24:   in 'constexpr' expansion of 'test_swap()'
x.ii:38:10: error: 'constexpr void expected<_Tp>::swap(expected<_Tp>&) [with
_Tp = int]' called in a constant expression
   38 |   e1.swap(e2);
      |   ~~~~~~~^~~~
x.ii:13:5: note: 'constexpr void expected<_Tp>::swap(expected<_Tp>&) [with _Tp
= int]' is not usable as a 'constexpr' function because:
   13 |     swap(expected& __x)
      |     ^~~~
x.ii:15:7: error: expression '<statement>' is not a constant expression
   15 |       if (this->has_value())
      |       ^~


The problem is that the _M_swap_val, _M_swap_unex and _M_swap_val_unex
functions are not constexpr, but the diagnostic doesn't make that obvious.

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-03-25
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
If it just said that the body of swap(expected&) was not a constant expression,
that would have been an improvement. I spent ages trying to understand what was
wrong with this line:

x.ii:15:7: error: expression '<statement>' is not a constant expression
   15 |       if (this->has_value())
      |       ^~

In fact the `if` statements are the only ones in the function that *are*
constant expressions, the problem is all the *other* statements. All the return
statements have non-constant operands.

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org

--- Comment #2 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Created attachment 52687
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52687&action=edit
elaborate non-constexpr if stmt branches

Seems we issued this error because we determined that neither branch of the
first 'if' is a valid constexpr body (since each branch will end up calling a
non-constexpr function), and so the 'if' is a non-constant "expression"
regardless of control flow, which makes 'swap' non-constexpr overall.

The attached diff makes us elaborate why neither branch of an 'if' is
non-constant in this case, similar to how we diagnose disjunctions during
satisfaction.  For this testcase we'd now get:

105050.C:43:24: error: non-constant condition for static assertion
   43 | static_assert(test_swap());
      |               ~~~~~~~~~^~
105050.C:43:24:   in ‘constexpr’ expansion of ‘test_swap()’
105050.C:38:10: error: ‘constexpr void expected<_Tp>::swap(expected<_Tp>&)
[with _Tp = int]’ called in a constant expression
   38 |   e1.swap(e2);
      |   ~~~~~~~^~~~
105050.C:13:5: note: ‘constexpr void expected<_Tp>::swap(expected<_Tp>&) [with
_Tp = int]’ is not usable as a ‘constexpr’ function because:
   13 |     swap(expected& __x)
      |     ^~~~
105050.C:15:7: note: neither branch of ‘if’ is a valid ‘constexpr’ body
because:
   15 |       if (this->has_value())
      |       ^~
105050.C:17:9: note: neither branch of ‘if’ is a valid ‘constexpr’ body
because:
   17 |         if (__x.has_value())
      |         ^~
105050.C:18:28: error: call to non-‘constexpr’ function ‘void
expected<_Tp>::_M_swap_val(expected<_Tp>&) [with _Tp = int]’
   18 |           this->_M_swap_val(__x);
      |           ~~~~~~~~~~~~~~~~~^~~~~
105050.C:4:10: note: ‘void expected<_Tp>::_M_swap_val(expected<_Tp>&) [with _Tp
= int]’ declared here
    4 |     void _M_swap_val(expected&) { }
      |          ^~~~~~~~~~~
105050.C:20:33: error: call to non-‘constexpr’ function ‘void
expected<_Tp>::_M_swap_val_unex(expected<_Tp>&) [with _Tp = int]’
   20 |           this->_M_swap_val_unex(__x);
      |           ~~~~~~~~~~~~~~~~~~~~~~^~~~~
105050.C:8:10: note: ‘void expected<_Tp>::_M_swap_val_unex(expected<_Tp>&)
[with _Tp = int]’ declared here
    8 |     void _M_swap_val_unex(expected&) { }
      |          ^~~~~~~~~~~~~~~~
105050.C:24:9: note: neither branch of ‘if’ is a valid ‘constexpr’ body
because:
   24 |         if (__x.has_value())
      |         ^~
105050.C:25:31: error: call to non-‘constexpr’ function ‘void
expected<_Tp>::_M_swap_val_unex(expected<_Tp>&) [with _Tp = int]’
   25 |           __x._M_swap_val_unex(*this);
      |           ~~~~~~~~~~~~~~~~~~~~^~~~~~~
105050.C:8:10: note: ‘void expected<_Tp>::_M_swap_val_unex(expected<_Tp>&)
[with _Tp = int]’ declared here
    8 |     void _M_swap_val_unex(expected&) { }
      |          ^~~~~~~~~~~~~~~~
105050.C:27:29: error: call to non-‘constexpr’ function ‘void
expected<_Tp>::_M_swap_unex(expected<_Tp>&) [with _Tp = int]’
   27 |           this->_M_swap_unex(__x);
      |           ~~~~~~~~~~~~~~~~~~^~~~~
105050.C:6:10: note: ‘void expected<_Tp>::_M_swap_unex(expected<_Tp>&) [with
_Tp = int]’ declared here
    6 |     void _M_swap_unex(expected&) { }
      |          ^~~~~~~~~~~~

We definitely should clarify the initial error, but I'm not sure if we want the
recursive elaboration, might be too noisy..

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think just "note: neither branch of ‘if’ is a valid ‘constexpr’ body" without
the "because ..." and the following notes would be good enough.

It tells the user to look at the 'if' body, not the condition on the line
highlighted by the caret. It's still OK for the caret to show the 'if' itself,
because the note now makes it clear that it's the *body* of that 'if' that is
the problem.

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
N.B. GCC 9 showed this instead of the "<statement>" part:

x.ii:15:7: error: expression
'(((expected<int>*)this)->expected<int>::has_value() ? ((&
__x)->expected<int>::has_value() ? ({...}) : ({...})) : ((&
__x)->expected<int>::has_value() ? ({...}) : ({...})))' is not a constant
expression
   15 |       if (this->has_value())
      |       ^~

That doesn't tell you the problem either, and is unreadable (and misleading
because there's no conditional expression in the original code). So shortening
that to just "<statement>" was definitely an improvement. I think your note
would make it even better.

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:ff465bd8a0f0f96a00d3067018442917b194b7af

commit r12-7831-gff465bd8a0f0f96a00d3067018442917b194b7af
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sat Mar 26 10:20:18 2022 -0400

    c++: diagnosing if-stmt with non-constant branches [PR105050]

    When an if-stmt is determined to be non-constant because both of its
    branches are non-constant, we issue a somewhat generic error which,
    since the error also points to the 'if' token, misleadingly suggests
    the condition is at fault:

      constexpr-105050.C:8:3: error: expression â<statement>â is not a
constant expression
          8 |   if (p != q && *p < 0)
            |   ^~

    This patch clarifies the error message to instead read:

      constexpr-105050.C:8:3: error: neither branch of âifâ is a constant
expression
          8 |   if (p != q && *p < 0)
            |   ^~

            PR c++/105050

    gcc/cp/ChangeLog:

            * constexpr.cc (potential_constant_expression_1) <case IF_STMT>:
            Clarify error message when a if-stmt is non-constant because its
            branches are non-constant.

    gcc/testsuite/ChangeLog:

            * g++.dg/cpp1y/constexpr-105050.C: New test.

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Thanks!

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think clang still gives a better error message here though:
<source>:43:15: error: static assertion expression is not an integral constant
expression
static_assert(test_swap());
              ^~~~~~~~~~~
<source>:18:17: note: non-constexpr function '_M_swap_val' cannot be used in a
constant expression
          this->_M_swap_val(__x);
                ^
<source>:38:6: note: in call to '&e1->swap(e2)'
  e1.swap(e2);
     ^
<source>:43:15: note: in call to 'test_swap()'
static_assert(test_swap());
              ^
<source>:4:10: note: declared here
    void _M_swap_val(expected&) { }
         ^

[Bug c++/105050] error: expression '<statement>' is not a constant expression

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105050

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement