public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/99287] New: std::source_location::function_name breaks constexpr
@ 2021-02-26 12:29 nickgray0 at brown dot edu
  2021-02-26 15:46 ` [Bug c++/99287] " jakub at gcc dot gnu.org
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: nickgray0 at brown dot edu @ 2021-02-26 12:29 UTC (permalink / raw)
  To: gcc-bugs

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

            Bug ID: 99287
           Summary: std::source_location::function_name breaks constexpr
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nickgray0 at brown dot edu
  Target Milestone: ---

the following code seems to break constexpr-ness for std::string_view::find

    #include <source_location>
    #include <string_view>

    template<typename>
    consteval auto f() {
        return std::string_view{
std::source_location::current().function_name() };
    }

    auto main()->int {
        constexpr auto s = f<int>();
        constexpr auto x = s.find("int");
    }



error message: In file included from
/opt/compiler-explorer/gcc-trunk-20210226/include/c++/11.0.1/string_view:781,
                 from <source>:2:
/opt/compiler-explorer/gcc-trunk-20210226/include/c++/11.0.1/bits/string_view.tcc:
In function 'int main()':
<source>:11:30:   in 'constexpr' expansion of
's.std::basic_string_view<char>::find(((const char*)"int"), 0)'
/opt/compiler-explorer/gcc-trunk-20210226/include/c++/11.0.1/string_view:384:26:
  in 'constexpr' expansion of '((const
std::basic_string_view<char>*)this)->std::basic_string_view<char>::find(__str,
__pos, std::char_traits<char>::length(__str))'
/opt/compiler-explorer/gcc-trunk-20210226/include/c++/11.0.1/bits/string_view.tcc:62:20:
error: '(((std::basic_string_view<char>::size_type)((&"consteval auto f() [with
<template-parameter-1-1> = int]"[56]) - ((&"consteval auto f() [with
<template-parameter-1-1> = int]"[21]) + 1))) > 2)' is not a constant expression
   62 |       while (__len >= __n)
      |              ~~~~~~^~~~~~



Replacing std::source_location with the legacy
std::experimental::source_location seems to fix the problem.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
@ 2021-02-26 15:46 ` jakub at gcc dot gnu.org
  2021-02-26 15:49 ` jakub at gcc dot gnu.org
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-02-26 15:46 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |jakub at gcc dot gnu.org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2021-02-26

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
To me this looks like type inconsistency somewhere.
The error is because fold_binary doesn't simplify
POINTER_DIFF_EXPR with ptrdiff_type and following 2 args:
 <addr_expr 0x7fffe929fc40
    type <pointer_type 0x7fffea06b498
        type <integer_type 0x7fffea06b3f0 char readonly public type_6 QI
            size <integer_cst 0x7fffea041fa8 constant 8>
            unit-size <integer_cst 0x7fffea041fc0 constant 1>
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea06b3f0 precision:8 min <integer_cst 0x7fffea062000 -128> max
<integer_cst 0x7fffea062030 127>
            pointer_to_this <pointer_type 0x7fffea06b498> reference_to_this
<reference_type 0x7fffe9537738>>
        public unsigned type_6 DI
        size <integer_cst 0x7fffea041eb8 constant 64>
        unit-size <integer_cst 0x7fffea041ed0 constant 8>
        align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea06b498
        pointer_to_this <pointer_type 0x7fffea1f2348>>
    constant
    arg:0 <array_ref 0x7fffe9082e70 type <integer_type 0x7fffea06b3f0 char>

        arg:0 <string_cst 0x7fffea1ae3c0 type <array_type 0x7fffe9078d20>
            readonly constant static "consteval auto f() [with
<template-parameter-1-1> = int]\000">
        arg:1 <integer_cst 0x7fffe931a528 constant 56>
       
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:59:43
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:59:36
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:59:50>
   
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:59:43
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:59:36
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:59:50>

and

 <pointer_plus_expr 0x7fffe9085c80
    type <pointer_type 0x7fffea06b498
        type <integer_type 0x7fffea06b3f0 char readonly public type_6 QI
            size <integer_cst 0x7fffea041fa8 constant 8>
            unit-size <integer_cst 0x7fffea041fc0 constant 1>
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea06b3f0 precision:8 min <integer_cst 0x7fffea062000 -128> max
<integer_cst 0x7fffea062030 127>
            pointer_to_this <pointer_type 0x7fffea06b498> reference_to_this
<reference_type 0x7fffe9537738>>
        public unsigned type_6 DI
        size <integer_cst 0x7fffea041eb8 constant 64>
        unit-size <integer_cst 0x7fffea041ed0 constant 8>
        align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea06b498
        pointer_to_this <pointer_type 0x7fffea1f2348>>
    constant
    arg:0 <nop_expr 0x7fffe92aba40
        type <pointer_type 0x7fffe9548bd0 type <integer_type 0x7fffe95485e8
char_type>
            public unsigned type_6 DI size <integer_cst 0x7fffea041eb8 64>
unit-size <integer_cst 0x7fffea041ed0 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea06b498>
        constant
        arg:0 <addr_expr 0x7fffe92ab9e0 type <pointer_type 0x7fffea06b498>
            constant
            arg:0 <array_ref 0x7fffe9087348 type <integer_type 0x7fffea06b3f0
char>

                arg:0 <string_cst 0x7fffea1ae3c0 type <array_type
0x7fffe9078d20>
                    readonly constant static "consteval auto f() [with
<template-parameter-1-1> = int]\000">
                arg:1 <integer_cst 0x7fffe930b300 constant 21>
               
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/char_traits.h:183:22
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/char_traits.h:183:18
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/char_traits.h:183:26>
           
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/char_traits.h:183:22
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/char_traits.h:183:18
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/char_traits.h:183:26>>
arg:1 <integer_cst 0x7fffea041fc0 1>>

and that seems to be because cxx_fold_pointer_plus_expression didn't do its job
as it only optimizes ADDR_EXPR ... plus constant, but in this case
we have NOP_EXPR of ADDR_EXPR ... plus constant.
As can be seen, the POINTER_PLUS_EXPR type is pointer to const char, but its
first operand does have a different type, pointer to char_type, i.e.
 <integer_type 0x7fffea06b3f0 char readonly public type_6 QI
    size <integer_cst 0x7fffea041fa8 type <integer_type 0x7fffea05f0a8
bitsizetype> constant 8>
    unit-size <integer_cst 0x7fffea041fc0 type <integer_type 0x7fffea05f000
sizetype> constant 1>
    align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea06b3f0 precision:8 min <integer_cst 0x7fffea062000 -128> max
<integer_cst 0x7fffea062030 127>
    pointer_to_this <pointer_type 0x7fffea06b498> reference_to_this
<reference_type 0x7fffe9537738>>
vs.
 <integer_type 0x7fffe95485e8 char_type readonly type_6 QI
    size <integer_cst 0x7fffea041fa8 type <integer_type 0x7fffea05f0a8
bitsizetype> constant 8>
    unit-size <integer_cst 0x7fffea041fc0 type <integer_type 0x7fffea05f000
sizetype> constant 1>
    align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea06b3f0 precision:8 min <integer_cst 0x7fffea062000 -128> max
<integer_cst 0x7fffea062030 127>
    pointer_to_this <pointer_type 0x7fffe9548bd0> reference_to_this
<reference_type 0x7fffe9548690>>
difference (the first one is the normal const char).

That char_type is in particular:
  template<typename _CharT>
    struct char_traits
    {
      typedef _CharT                                    char_type;
from bits/char_traits.h.

I thought in the FE we require that binary expressions other than
shifts/rotates etc. have really the same types rather than just compatible
types.

I see the problematic MODIFY_EXPR with lhs VIEW_CONVERT_EXPR<const char
*>(__first) with the pointer to const char type and rhs
std::char_traits<char>::find (VIEW_CONVERT_EXPR<const char *>(__first),
(VIEW_CONVERT_EXPR<size_type>(__len) - VIEW_CONVERT_EXPR<size_type>(__n)) + 1,
(const char_type &) &__elem0); (with the pointer to char_type type) created in
tsubst_copy_and_build:
19973           warning_sentinel s(warn_div_by_zero);
19974           tree lhs = RECUR (TREE_OPERAND (t, 0));
19975           tree rhs = RECUR (TREE_OPERAND (t, 2));
19976           tree r = build_x_modify_expr
19977             (EXPR_LOCATION (t), lhs, TREE_CODE (TREE_OPERAND (t, 1)),
rhs,
19978              complain|decltype_flag);
19979           /* TREE_NO_WARNING must be set if either the expression was
19980              parenthesized or it uses an operator such as >>= rather
19981              than plain assignment.  In the former case, it was already
19982              set and must be copied.  In the latter case,
TREE_OPERAND (t, 0) has
    type <pointer_type 0x7fffe906cc78
        type <template_type_parm 0x7fffe906cbd0 _CharT readonly type_0 type_6
VOID ...>>
type and TREE_OPERAND (t, 2) doesn't have any - it is:
 <call_expr 0x7fffe92e2048
    fn <scope_ref 0x7fffe93f1c08 tree_0
        arg:0 <template_type_parm 0x7fffe9387738 traits_type tree_0 type_0
type_6 VOID
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea1d2d20
           index 1 level 1 orig_level 1
            chain <type_decl 0x7fffe9379850 _Traits>>
        arg:1 <identifier_node 0x7fffea1bba40 find
            normal local bindings <(nil)>>
       
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:27
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:27
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:30>
    arg:0 <view_convert_expr 0x7fffe923d4e0
        type <pointer_type 0x7fffe906cc78 type <template_type_parm
0x7fffe906cbd0 _CharT>
            unsigned type_0 type_6 DI
            size <integer_cst 0x7fffea041eb8 constant 64>
            unit-size <integer_cst 0x7fffea041ed0 constant 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffe9a5a000>
        public
        arg:0 <var_decl 0x7fffe9238ea0 __first type <pointer_type
0x7fffe906cc78>
            used tree_6 unsigned in_system_header decl_5 DI
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:58:21
size <integer_cst 0x7fffea041eb8 64> unit-size <integer_cst 0x7fffea041ed0 8>
            align:64 warn_if_not_align:0 context <function_decl 0x7fffe939d700
find> initial <plus_expr 0x7fffe93f1938> chain <var_decl 0x7fffe9238f30
__last>>
       
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:32
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:32
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:38>
    arg:1 <plus_expr 0x7fffe93f1d20
        type <integer_type 0x7fffe9387348 size_type public unsigned type_6 DI
size <integer_cst 0x7fffea041eb8 64> unit-size <integer_cst 0x7fffea041ed0 8>
            align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea05f7e0 precision:64 min <integer_cst 0x7fffea062198 0> max <integer_cst
0x7fffea042620 18446744073709551615>
            pointer_to_this <pointer_type 0x7fffe93ac498> reference_to_this
<reference_type 0x7fffe906f498>>

        arg:0 <minus_expr 0x7fffe93f1c80 type <integer_type 0x7fffe9387348
size_type>

            arg:0 <view_convert_expr 0x7fffe923d500 type <integer_type
0x7fffe9387348 size_type>
                public arg:0 <var_decl 0x7fffe9599000 __len>
               
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:41
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:41
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:45>
            arg:1 <view_convert_expr 0x7fffe923d520 type <integer_type
0x7fffe9387348 size_type>
                public arg:0 <parm_decl 0x7fffe906e200 __n>
               
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:49
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:49
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:51>
           
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:47
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:41
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:51>
        arg:1 <non_lvalue_expr 0x7fffe923d6c0 type <integer_type 0x7fffea05f5e8
int>
            constant public
            arg:0 <integer_cst 0x7fffea062270 constant 1>
           
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:55
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:55
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:55>
       
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:53
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:41
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:55>
    arg:2 <view_convert_expr 0x7fffe923d820
        type <template_type_parm 0x7fffe906cbd0 _CharT readonly type_0 type_6
VOID
            align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
0x7fffea2125e8
           index 0 level 1 orig_level 1
            pointer_to_this <pointer_type 0x7fffe906cc78>>
        public
        arg:0 <var_decl 0x7fffe9238e10 __elem0 type <template_type_parm
0x7fffe906cbd0 _CharT>
            used tree_2 tree_6 in_system_header decl_5 VOID
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:57:20
            align:8 warn_if_not_align:0 context <function_decl 0x7fffe939d700
find> initial <array_ref 0x7fffe904d5b0> chain <var_decl 0x7fffe9238ea0
__first>>
       
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:58
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:58
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:64>
   
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:31
start:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:14
finish:
/home/jakub/src/gcc/obj36i/usr/local/include/c++/11.0.1/bits/string_view.tcc:65:65>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
  2021-02-26 15:46 ` [Bug c++/99287] " jakub at gcc dot gnu.org
@ 2021-02-26 15:49 ` jakub at gcc dot gnu.org
  2021-03-05 15:30 ` ppalka at gcc dot gnu.org
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-02-26 15:49 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That t (MODOP_EXPR) is in the source:
          // Find the first occurrence of __elem0:
          __first = traits_type::find(__first, __len - __n + 1, __elem0);

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
  2021-02-26 15:46 ` [Bug c++/99287] " jakub at gcc dot gnu.org
  2021-02-26 15:49 ` jakub at gcc dot gnu.org
@ 2021-03-05 15:30 ` ppalka at gcc dot gnu.org
  2021-03-05 15:39 ` jakub at gcc dot gnu.org
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-03-05 15:30 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #3 from Patrick Palka <ppalka at gcc dot gnu.org> ---
IIUC, those two types are actually the same, it's just that one of them was
obtained through the char_type alias, and it seems debug_tree prefers to show
the name of the alias when the type came from one.

It seems the issue ultimately is in cxx_eval_increment_expression: during
evaluation of ++__first with __first = &"mystr"[n] + m and with lval=false,
since cxx_fold_pointer_plus_expression and not fold_build2 is responsible for
simplifying this POINTER_PLUS_EXPR to &"mystr"[n+m], returning 'mod' actually
returns the unreduced &"mystr"[n] + m rather than the reduced &"mystr"[n+m]
that we obtain as part of constexpr evaluation of the temporary MODIFY_EXPR. 
This unreduced return value of cxx_eval_increment interfers with later
constexpr evaluation, e.g. folding of the POINTER_DIFF_EXPR.

I'm testing the following which updates 'mod' with the result of evaluation of
the MODIFY_EXPR:

--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -5582,20 +5582,14 @@ cxx_eval_increment_expression (const constexpr_ctx
*ctx, tree t,
   /* Storing the modified value.  */
   tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
                           MODIFY_EXPR, type, op, mod);
-  cxx_eval_constant_expression (ctx, store,
-                               true, non_constant_p, overflow_p);
+  mod = cxx_eval_constant_expression (ctx, store, false,
+                                     non_constant_p, overflow_p);
   ggc_free (store);

   /* And the value of the expression.  */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
                   ` (2 preceding siblings ...)
  2021-03-05 15:30 ` ppalka at gcc dot gnu.org
@ 2021-03-05 15:39 ` jakub at gcc dot gnu.org
  2021-03-05 15:57 ` jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-03-05 15:39 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Patrick Palka from comment #3)
> IIUC, those two types are actually the same, it's just that one of them was
> obtained through the char_type alias, and it seems debug_tree prefers to
> show the name of the alias when the type came from one.
> 
> It seems the issue ultimately is in cxx_eval_increment_expression: during
> evaluation of ++__first with __first = &"mystr"[n] + m and with lval=false,
> since cxx_fold_pointer_plus_expression and not fold_build2 is responsible
> for simplifying this POINTER_PLUS_EXPR to &"mystr"[n+m], returning 'mod'
> actually returns the unreduced &"mystr"[n] + m rather than the reduced
> &"mystr"[n+m] that we obtain as part of constexpr evaluation of the
> temporary MODIFY_EXPR.  This unreduced return value of cxx_eval_increment
> interfers with later constexpr evaluation, e.g. folding of the
> POINTER_DIFF_EXPR.
> 
> I'm testing the following which updates 'mod' with the result of evaluation
> of the MODIFY_EXPR:
> 
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -5582,20 +5582,14 @@ cxx_eval_increment_expression (const constexpr_ctx
> *ctx, tree t,
>    /* Storing the modified value.  */
>    tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
>                            MODIFY_EXPR, type, op, mod);
> -  cxx_eval_constant_expression (ctx, store,
> -                               true, non_constant_p, overflow_p);
> +  mod = cxx_eval_constant_expression (ctx, store, false,
> +                                     non_constant_p, overflow_p);
>    ggc_free (store);

Maybe; I'm a little bit worried here though because
cxx_eval_constant_expression
can in various cases return the tree passed to it on failure (i.e. the store)
or can return NULL_TREE.
And store is then ggc_freed, so shouldn't be really used afterwards.
So perhaps
  tree new_mod = cxx_eval_constant_expression (ctx, store, false,
non_constant_p, overflow_p);
  if (new_mod && new_mod != store)
    mod = new_mod;

  ggc_free (store);
?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
                   ` (3 preceding siblings ...)
  2021-03-05 15:39 ` jakub at gcc dot gnu.org
@ 2021-03-05 15:57 ` jakub at gcc dot gnu.org
  2021-03-05 16:08 ` ppalka at gcc dot gnu.org
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-03-05 15:57 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Or perhaps another option would be instead of
return mod;
do
return cxx_eval_constant_expression (ctx, mod, false, non_constant_p,
overflow_p);
?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
                   ` (4 preceding siblings ...)
  2021-03-05 15:57 ` jakub at gcc dot gnu.org
@ 2021-03-05 16:08 ` ppalka at gcc dot gnu.org
  2021-03-06 22:10 ` cvs-commit at gcc dot gnu.org
  2021-03-06 22:12 ` ppalka at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-03-05 16:08 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> (In reply to Patrick Palka from comment #3)
> > IIUC, those two types are actually the same, it's just that one of them was
> > obtained through the char_type alias, and it seems debug_tree prefers to
> > show the name of the alias when the type came from one.
> > 
> > It seems the issue ultimately is in cxx_eval_increment_expression: during
> > evaluation of ++__first with __first = &"mystr"[n] + m and with lval=false,
> > since cxx_fold_pointer_plus_expression and not fold_build2 is responsible
> > for simplifying this POINTER_PLUS_EXPR to &"mystr"[n+m], returning 'mod'
> > actually returns the unreduced &"mystr"[n] + m rather than the reduced
> > &"mystr"[n+m] that we obtain as part of constexpr evaluation of the
> > temporary MODIFY_EXPR.  This unreduced return value of cxx_eval_increment
> > interfers with later constexpr evaluation, e.g. folding of the
> > POINTER_DIFF_EXPR.
> > 
> > I'm testing the following which updates 'mod' with the result of evaluation
> > of the MODIFY_EXPR:
> > 
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -5582,20 +5582,14 @@ cxx_eval_increment_expression (const constexpr_ctx
> > *ctx, tree t,
> >    /* Storing the modified value.  */
> >    tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
> >                            MODIFY_EXPR, type, op, mod);
> > -  cxx_eval_constant_expression (ctx, store,
> > -                               true, non_constant_p, overflow_p);
> > +  mod = cxx_eval_constant_expression (ctx, store, false,
> > +                                     non_constant_p, overflow_p);
> >    ggc_free (store);
> 
> Maybe; I'm a little bit worried here though because
> cxx_eval_constant_expression
> can in various cases return the tree passed to it on failure (i.e. the
> store) or can return NULL_TREE.
> And store is then ggc_freed, so shouldn't be really used afterwards.
> So perhaps
>   tree new_mod = cxx_eval_constant_expression (ctx, store, false,
> non_constant_p, overflow_p);
>   if (new_mod && new_mod != store)
>     mod = new_mod;
> 
>   ggc_free (store);
> ?

Good point.  Though I don't see how cxx_eval_constant_expression could
plausibly return NULL_TREE here since we're dealing with a MODIFY_EXPR?  And I
think checking *non_constant_p could subsume checking new_mod != store.  So
perhaps just

@@ -5582,20 +5582,16 @@ cxx_eval_increment_expression (const constexpr_ctx
*ctx, tree t,
   /* Storing the modified value.  */
   tree store = build2_loc (cp_expr_loc_or_loc (t, input_location),
                           MODIFY_EXPR, type, op, mod);
-  cxx_eval_constant_expression (ctx, store,
-                               true, non_constant_p, overflow_p);
+  mod = cxx_eval_constant_expression (ctx, store, false,
+                                     non_constant_p, overflow_p);
   ggc_free (store);
+  if (*non_constant_p)
+    return t;

would cover our bases.

(In reply to Jakub Jelinek from comment #5)
> Or perhaps another option would be instead of
> return mod;
> do
> return cxx_eval_constant_expression (ctx, mod, false, non_constant_p,
> overflow_p);
> ?

I think should work, though it'd mean we'd be redundantly evaluating 'mod'
twice in the !lval case I suppose.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
                   ` (5 preceding siblings ...)
  2021-03-05 16:08 ` ppalka at gcc dot gnu.org
@ 2021-03-06 22:10 ` cvs-commit at gcc dot gnu.org
  2021-03-06 22:12 ` ppalka at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-03-06 22:10 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 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:d1bba463bd0d5692b7fa58ee37a61a55b2517456

commit r11-7546-gd1bba463bd0d5692b7fa58ee37a61a55b2517456
Author: Patrick Palka <ppalka@redhat.com>
Date:   Sat Mar 6 17:09:07 2021 -0500

    c++: Fix constexpr evaluation of pre-increment when !lval [PR99287]

    Here, during cxx_eval_increment_expression (with lval=false) of
    ++__first where __first is &"mystr"[0], we correctly update __first
    to &"mystr"[1] but we end up returning &"mystr"[0] + 1 instead of
    &"mystr"[1].  This unreduced return value inhibits other pointer
    arithmetic folding during later constexpr evaluation, which ultimately
    causes the constexpr evaluation to fail.

    It turns out the simplification of &"mystr"[0] + 1 to &"mystr"[1]
    is performed by cxx_fold_pointer_plus_expression, not by fold_build2.
    So we perform this simplification during constexpr evaluation of
    the temporary MODIFY_EXPR (during which we assign to __first the
    simplified value), but then we return 'mod' which has only been folded
    via fold_build2 and hasn't gone through cxx_fold_pointer_plus_expression.

    This patch fixes this by updating 'mod' with the result of the
    MODIFY_EXPR evaluation appropriately, so that it captures any additional
    folding of the expression when !lval.  We now need to be wary of this
    evaluation failing and returning e.g. the MODIFY_EXPR or NULL_TREE; it
    seems checking *non_constant_p should cover our bases here and is
    generally prudent.

    gcc/cp/ChangeLog:

            PR c++/99287
            * constexpr.c (cxx_eval_increment_expression): Pass lval when
            evaluating the MODIFY_EXPR, and update 'mod' with the result of
            this evaluation.  Check *non_constant_p afterwards.  For prefix
            ops, just return 'mod'.

    gcc/testsuite/ChangeLog:

            PR c++/99287
            * g++.dg/cpp2a/constexpr-99287.C: New test.

    Co-authored-by: Jakub Jelinek <jakub@redhat.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Bug c++/99287] std::source_location::function_name breaks constexpr
  2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
                   ` (6 preceding siblings ...)
  2021-03-06 22:10 ` cvs-commit at gcc dot gnu.org
@ 2021-03-06 22:12 ` ppalka at gcc dot gnu.org
  7 siblings, 0 replies; 9+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-03-06 22:12 UTC (permalink / raw)
  To: gcc-bugs

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
   Target Milestone|---                         |11.0

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 11.  Thanks for the bug report!

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-06 22:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-26 12:29 [Bug c++/99287] New: std::source_location::function_name breaks constexpr nickgray0 at brown dot edu
2021-02-26 15:46 ` [Bug c++/99287] " jakub at gcc dot gnu.org
2021-02-26 15:49 ` jakub at gcc dot gnu.org
2021-03-05 15:30 ` ppalka at gcc dot gnu.org
2021-03-05 15:39 ` jakub at gcc dot gnu.org
2021-03-05 15:57 ` jakub at gcc dot gnu.org
2021-03-05 16:08 ` ppalka at gcc dot gnu.org
2021-03-06 22:10 ` cvs-commit at gcc dot gnu.org
2021-03-06 22:12 ` ppalka at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).