On 9/24/24 12:53 PM, Jakub Jelinek wrote:
On Mon, Sep 23, 2024 at 03:46:36PM -0400, Jason Merrill wrote:
-frange-based-for-ext-temps
or do you have better suggestion?

I'd probably drop "based", "range-for" seems enough.

Shall we allow also disabling it in C++23 or later modes, or override
user choice unconditionally for C++23+ and only allow users to
enable/disable it in C++11-C++20?

Hmm, I think the latter.

What about the __cpp_range_based_for predefined macro?
Shall it be defined to the C++23 202211L value if the switch is on?
While that could be done in theory for C++17 and later code, for C++11/14
__cpp_range_based_for is 200907L and doesn't include the C++17
201603L step.  Or keep the macro only for C++23 and later?

I think update the macro for 17 and later.

Ok.

Here is a new patch.

@@ -44600,11 +44609,14 @@ cp_convert_omp_range_for (tree &this_pre
          else
        {
          range_temp = build_range_temp (init);
+         tree name = DECL_NAME (range_temp);
          DECL_NAME (range_temp) = NULL_TREE;
          pushdecl (range_temp);
+         DECL_NAME (range_temp) = name;
          cp_finish_decl (range_temp, init,
                          /*is_constant_init*/false, NULL_TREE,
                          LOOKUP_ONLYCONVERTING);
+         DECL_NAME (range_temp) = NULL_TREE;

This messing with the name needs a rationale.  What wants it to be null?

I'll add comments.  The first = NULL_TREE; is needed so that pushdecl
doesn't register the temporary for name lookup, the = name now is so that
cp_finish_decl recognizes the temporary as range based for temporary
for the lifetime extension, and the last one is just to preserve previous
behavior, not have it visible in debug info etc.

But cp_convert_range_for doesn't ever set the name to NULL_TREE, why should
the OMP variant be different?

Having it visible to name lookup in the debugger seems beneficial. Having it
visible to the code seems less useful, but not important to prevent.

So, in the end it works fine even for the OpenMP case when not inside of a
template, all I had to add is the renaming of the symbol at the end after
pop_scope from "__for_range " to "__for_range" etc.
It doesn't work unfortunately during instantiation, we only create a single
scope in that case for the whole loop nest rather than one for each loop in
it and changing that isn't easy.  With the "__for_range " name in, if there
are 2+ range based for loops in the OpenMP loop nest (collapsed or ordered),
one gets then errors about defining it multiple times.
I'll try to fix that up at incrementally later, for now I just went with
a new flag to the function, so that it does the DECL_NAME dances only when
called from the instantiation (and confirmed actually all 3 spots are
needed, clearing before pushdecl, resetting back before cp_finish_decl and
clearing after cp_finish_decl, the last one so that pop_scope doesn't ICE
on seeing the name change).

Don't worry too much about fixing it up if it's complicated.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-09-24  Jakub Jelinek  <ja...@redhat.com>

        PR c++/107637
gcc/
        * omp-general.cc (find_combined_omp_for, find_nested_loop_xform):
        Handle CLEANUP_POINT_EXPR like TRY_FINALLY_EXPR.
        * doc/invoke.texi (frange-for-ext-temps): Document.  Add
        -fconcepts to the C++ option list.
gcc/c-family/
        * c.opt (frange-for-ext-temps): New option.
        * c-opts.cc (c_common_post_options): Set flag_range_for_ext_temps
        for C++23 or later or for C++11 or later in !flag_iso mode if
        the option wasn't set by user.
        * c-cppbuiltin.cc (c_cpp_builtins): Change __cpp_range_based_for
        value for flag_range_for_ext_temps from 201603L to 202212L in C++17
        or later.
        * c-omp.cc (c_find_nested_loop_xform_r): Handle CLEANUP_POINT_EXPR
        like TRY_FINALLY_EXPR.
gcc/cp/
        * cp-tree.h: Implement C++23 P2718R0 - Wording for P2644R1 Fix for
        Range-based for Loop.
        (cp_convert_omp_range_for): Add bool tmpl_p argument.
        (find_range_for_decls): Declare.
        * parser.cc (cp_convert_range_for): For flag_range_for_ext_temps call
        push_stmt_list () before cp_finish_decl for range_temp and save it
        temporarily to FOR_INIT_STMT.
        (cp_convert_omp_range_for): Add tmpl_p argument.  If set, remember
        DECL_NAME of range_temp and for cp_finish_decl call restore it before
        clearing it again, if unset, don't adjust DECL_NAME of range_temp at
        all.
        (cp_parser_omp_loop_nest): For flag_range_for_ext_temps range for add
        CLEANUP_POINT_EXPR around sl.  Call find_range_for_decls and adjust
        DECL_NAMEs for range fors if not processing_template_decl.  Adjust
        cp_convert_omp_range_for caller.  Remove superfluous backslash at the
        end of line.
        * decl.cc (initialize_local_var): For flag_range_for_ext_temps
        temporarily clear stmts_are_full_exprs_p rather than set for
        for_range__identifier decls.
        * call.cc (extend_ref_init_temps): For flag_range_for_ext_temps return
        init early for for_range__identifier decls.
        * semantics.cc (find_range_for_decls): New function.
        (finish_for_stmt): Use it.  For flag_range_for_ext_temps if
        cp_convert_range_for set FOR_INIT_STMT, pop_stmt_list it and wrap
        into CLEANUP_POINT_EXPR.
        * pt.cc (tsubst_omp_for_iterator): Adjust tsubst_omp_for_iterator
        caller.
        (tsubst_stmt) <case OMP_FOR>: For flag_range_for_ext_temps if there
        are any range fors in the loop nest, add push_stmt_list starting
        before the initializations, pop_stmt_list it after the body and wrap
        into CLEANUP_POINT_EXPR.  Change DECL_NAME of range for temps from
        NULL to for_range_identifier.
gcc/testsuite/
        * g++.dg/cpp23/range-for1.C: New test.
        * g++.dg/cpp23/range-for2.C: New test.
        * g++.dg/cpp23/range-for3.C: New test.
        * g++.dg/cpp23/range-for4.C: New test.
        * g++.dg/cpp23/range-for5.C: New test.
        * g++.dg/cpp23/range-for6.C: New test.
        * g++.dg/cpp23/range-for7.C: New test.
        * g++.dg/cpp23/range-for8.C: New test.
        * g++.dg/cpp23/feat-cxx2b.C (__cpp_range_based_for): Check for
        202212L rather than 201603L.
        * g++.dg/cpp26/feat-cxx26.C (__cpp_range_based_for): Likewise.
        * g++.dg/warn/Wdangling-reference4.C: Don't expect warning for C++23
        or newer.  Use dg-additional-options rather than dg-options.
libgomp/
        * testsuite/libgomp.c++/range-for-1.C: New test.
        * testsuite/libgomp.c++/range-for-2.C: New test.
        * testsuite/libgomp.c++/range-for-3.C: New test.
        * testsuite/libgomp.c++/range-for-4.C: New test.
        * testsuite/libgomp.c++/range-for-5.C: New test.

--- gcc/omp-general.cc.jj       2024-09-24 11:31:48.775621337 +0200
+++ gcc/omp-general.cc  2024-09-24 11:37:02.761332388 +0200
@@ -972,6 +972,7 @@ find_combined_omp_for (tree *tp, int *wa
        *walk_subtrees = 1;
        break;
      case TRY_FINALLY_EXPR:
+    case CLEANUP_POINT_EXPR:
        pdata[0] = tp;
        *walk_subtrees = 1;
        break;
@@ -4164,6 +4165,7 @@ find_nested_loop_xform (tree *tp, int *w
        *walk_subtrees = 1;
        break;
      case TRY_FINALLY_EXPR:
+    case CLEANUP_POINT_EXPR:
        pdata[0] = tp;
        *walk_subtrees = 1;
        break;
--- gcc/doc/invoke.texi.jj      2024-09-24 11:31:48.695622430 +0200
+++ gcc/doc/invoke.texi 2024-09-24 12:09:08.908039208 +0200
@@ -214,7 +214,7 @@ in the following sections.
  @xref{C++ Dialect Options,,Options Controlling C++ Dialect}.
  @gccoptlist{-fabi-version=@var{n}  -fno-access-control
  -faligned-new=@var{n}  -fargs-in-order=@var{n}  -fchar8_t  -fcheck-new
--fconstexpr-depth=@var{n}  -fconstexpr-cache-depth=@var{n}
+-fconcepts  -fconstexpr-depth=@var{n}  -fconstexpr-cache-depth=@var{n}
  -fconstexpr-loop-limit=@var{n}  -fconstexpr-ops-limit=@var{n}
  -fno-elide-constructors
  -fno-enforce-eh-specs
@@ -233,7 +233,7 @@ in the following sections.
  -fnew-ttp-matching
  -fno-nonansi-builtins  -fnothrow-opt  -fno-operator-names
  -fno-optional-diags
--fno-pretty-templates
+-fno-pretty-templates  -frange-for-ext-temps
  -fno-rtti  -fsized-deallocation
  -ftemplate-backtrace-limit=@var{n}
  -ftemplate-depth=@var{n}
@@ -3614,6 +3614,15 @@ the default template arguments for that
  behaviors make it harder to understand the error message rather than
  easier, you can use @option{-fno-pretty-templates} to disable them.
+@opindex frange-for-ext-temps
+@item -frange-for-ext-temps
+Enable lifetime extension of C++ range based for temporaries.
+With @option{-std=c++23} and above this is part of the language standard,
+so lifetime of the temporaries is extended until the end of the loop
+regardless of this option.  This option allows enabling that behavior also
+in earlier versions of the standard and is enabled by default in the
+GNU dialects, from @option{-std=gnu++11} until @option{-std=gnu++20}.
+
  @opindex fno-rtti
  @opindex frtti
  @item -fno-rtti
--- gcc/c-family/c.opt.jj       2024-09-24 11:31:37.380776989 +0200
+++ gcc/c-family/c.opt  2024-09-24 11:49:23.856220260 +0200
@@ -2209,6 +2209,10 @@ fprintf-return-value
  C ObjC C++ ObjC++ LTO Optimization Var(flag_printf_return_value) Init(1)
  Treat known sprintf return values as constants.
+frange-for-ext-temps
+C++ ObjC++ Var(flag_range_for_ext_temps)
+Enable lifetime extension of range based for temporaries.
+
  freplace-objc-classes
  ObjC ObjC++ LTO Var(flag_replace_objc_classes)
  Used in Fix-and-Continue mode to indicate that object files may be swapped in 
at runtime.
--- gcc/c-family/c-opts.cc.jj   2024-09-24 11:31:37.344777481 +0200
+++ gcc/c-family/c-opts.cc      2024-09-24 13:36:47.135322261 +0200
@@ -1160,6 +1160,16 @@ c_common_post_options (const char **pfil
    if (cxx_dialect >= cxx20)
      flag_concepts = 1;
+ /* Enable lifetime extension of range based for temporaries for C++23
+     regardless of command line setting.  */
+  if (cxx_dialect >= cxx23)
+    flag_range_for_ext_temps = 1;

Let's also give an error for trying to disable it in C++23+.

+++ gcc/cp/semantics.cc 2024-09-24 12:20:35.083694190 +0200
@@ -1637,6 +1637,31 @@ finish_for_expr (tree expr, tree for_stm
    FOR_EXPR (for_stmt) = expr;
  }

Missing function comment, maybe just use the one below?

+void
+find_range_for_decls (tree range_for_decl[3])
+{
+  /* During parsing of the body, range for uses "__for_{range,begin,end} "
+     decl names to make those unaccessible by code in the body.
+     Change it to ones with underscore instead of space, so that it can
+     be inspected in the debugger.  */

--- gcc/testsuite/g++.dg/cpp23/range-for3.C.jj  2024-09-24 13:46:42.151189765 
+0200
+++ gcc/testsuite/g++.dg/cpp23/range-for3.C     2024-09-24 13:47:26.571582778 
+0200
@@ -0,0 +1,6 @@
+// P2718R0 - Wording for P2644R1 Fix for Range-based for Loop
+// { dg-do run { target c++11 } }
+// { dg-options "" }

Please add a comment to this and range-for4 explaining that this is to get the fix enabled in GNU modes.

OK with those changes.

Jason

Reply via email to