On 11/29/22 07:32, Jakub Jelinek wrote:
Hi!

The PR84469 patch I've just posted regresses the for-21.C testcase,
when in OpenMP loop there are at least 2 associated loops and
in a template outer structured binding with non type dependent expression
is used in the expressions of some inner loop, we don't diagnose those
any longer, as the (weirdly worded) diagnostics was only done during
finish_id_expression -> mark_used which for the inner loop expressions
happens before the structured bindings are finalized.  When in templates,
mark_used doesn't diagnose uses of non-deduced variables, and if the
range for expression is type dependent, it is similarly diagnosed during
instantiation.  But newly with the PR84469 fix if the range for expression
is not type dependent, there is no place that would diagnose it, as during
instantiation the structured bindings are already deduced.

The following patch diagnoses it in that case during finish_omp_for (for
consistency with the same weird message).

I'll commit this to trunk if the other patch is approved and it passes
bootstrap/regtest.

2022-11-29  Jakub Jelinek  <ja...@redhat.com>

        PR c++/84469
        * semantics.cc: Define INCLUDE_MEMORY before including system.h.
        (struct finish_omp_for_data): New type.
        (finish_omp_for_decomps_r): New function.
        (finish_omp_for): Diagnose uses of non-type-dependent range for
        loop decompositions in inner OpenMP associated loops in templates.

        * g++.dg/gomp/for-21.C (f6): Adjust lines of expected diagnostics.
        * g++.dg/gomp/for-22.C: New test.

--- gcc/cp/semantics.cc.jj      2022-11-19 09:21:14.897436616 +0100
+++ gcc/cp/semantics.cc 2022-11-29 12:58:36.165771985 +0100
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.
  <http://www.gnu.org/licenses/>.  */
#include "config.h"
+#define INCLUDE_MEMORY
  #include "system.h"
  #include "coretypes.h"
  #include "target.h"
@@ -10401,6 +10402,47 @@ handle_omp_for_class_iterator (int i, lo
    return false;
  }
+struct finish_omp_for_data {
+  std::unique_ptr<hash_set<tree>> decomps;
+  bool fail;
+  location_t loc;
+};
+
+/* Helper function for finish_omp_for.  Diagnose uses of structured
+   bindings of OpenMP collapsed loop range for loops in the associated
+   loops.  If not processing_template_decl, this is diagnosed by
+   finish_id_expression -> mark_used before the range for is deduced.
+   And if processing_template_decl and the range for expression is
+   type dependent, it is similarly diagnosed during instantiation.
+   Only when processing_template_decl and range for expression is
+   not type dependent, we wouldn't diagnose it at all, so do it
+   from finish_omp_for in that case.  */
+
+static tree
+finish_omp_for_decomps_r (tree *tp, int *, void *d)
+{
+  if (VAR_P (*tp)
+      && DECL_DECOMPOSITION_P (*tp)
+      && !type_dependent_expression_p (*tp)
+      && DECL_HAS_VALUE_EXPR_P (*tp))
+    {
+      tree v = DECL_VALUE_EXPR (*tp);
+      if (TREE_CODE (v) == ARRAY_REF
+         && VAR_P (TREE_OPERAND (v, 0))
+         && DECL_DECOMPOSITION_P (TREE_OPERAND (v, 0)))
+       {
+         finish_omp_for_data *data = (finish_omp_for_data *) d;
+         if (data->decomps->contains (TREE_OPERAND (v, 0)))
+           {
+             error_at (data->loc, "use of %qD before deduction of %<auto%>",
+                       *tp);
+             data->fail = true;
+           }
+       }
+    }
+  return NULL_TREE;
+}
+
  /* Build and validate an OMP_FOR statement.  CLAUSES, BODY, COND, INCR
     are directly for their associated operands in the statement.  DECL
     and INIT are a combo; if DECL is NULL then INIT ought to be a
@@ -10419,6 +10461,7 @@ finish_omp_for (location_t locus, enum t
    int i;
    int collapse = 1;
    int ordered = 0;
+  finish_omp_for_data data;
gcc_assert (TREE_VEC_LENGTH (declv) == TREE_VEC_LENGTH (initv));
    gcc_assert (TREE_VEC_LENGTH (declv) == TREE_VEC_LENGTH (condv));
@@ -10479,7 +10522,25 @@ finish_omp_for (location_t locus, enum t
        elocus = EXPR_LOCATION (init);
if (cond == global_namespace)
-       continue;
+       {
+         gcc_assert (processing_template_decl);
+         if (TREE_VEC_LENGTH (declv) > 1
+             && VAR_P (decl)
+             && DECL_DECOMPOSITION_P (decl)
+             && !type_dependent_expression_p (decl))
+           {
+             gcc_assert (DECL_HAS_VALUE_EXPR_P (decl));
+             tree v = DECL_VALUE_EXPR (decl);
+             gcc_assert (TREE_CODE (v) == ARRAY_REF
+                         && VAR_P (TREE_OPERAND (v, 0))
+                         && DECL_DECOMPOSITION_P (TREE_OPERAND (v, 0)));
+             if (!data.decomps)
+               data.decomps
+                 = std::unique_ptr<hash_set<tree>> (new hash_set<tree>);
+             data.decomps->add (TREE_OPERAND (v, 0));
+           }
+         continue;
+       }
if (cond == NULL)
        {
@@ -10497,6 +10558,37 @@ finish_omp_for (location_t locus, enum t
        TREE_VEC_ELT (initv, i) = init;
      }
+ if (data.decomps)
+    {
+      data.fail = false;
+      data.loc = locus;
+      hash_set<tree> pset;
+      for (i = 0; i < TREE_VEC_LENGTH (declv); i++)
+       {
+         init = TREE_VEC_ELT (initv, i);
+         cond = TREE_VEC_ELT (condv, i);
+         incr = TREE_VEC_ELT (incrv, i);
+         data.loc = EXPR_LOC_OR_LOC (init, locus);
+         cp_walk_tree (&init, finish_omp_for_decomps_r, &data, &pset);
+         data.loc = EXPR_LOC_OR_LOC (cond, locus);
+         cp_walk_tree (&cond, finish_omp_for_decomps_r, &data, &pset);
+         data.loc = EXPR_LOC_OR_LOC (incr, locus);
+         cp_walk_tree (&incr, finish_omp_for_decomps_r, &data, &pset);
+       }
+      if (orig_inits && !data.fail)
+       {
+         tree orig_init;
+         FOR_EACH_VEC_ELT (*orig_inits, i, orig_init)
+           {
+             data.loc = EXPR_LOC_OR_LOC (orig_init, locus);
+             cp_walk_tree (&orig_init, finish_omp_for_decomps_r,
+                           &data, &pset);
+           }
+       }
+      if (data.fail)
+       return NULL;
+    }
+
    if (orig_inits)
      {
        bool fail = false;
--- gcc/testsuite/g++.dg/gomp/for-21.C.jj       2020-01-12 11:54:37.178401867 
+0100
+++ gcc/testsuite/g++.dg/gomp/for-21.C  2022-11-29 13:06:59.038410557 +0100
@@ -54,9 +54,9 @@ void
  f6 (S (&a)[10])
  {
    #pragma omp for collapse (2)
-  for (auto [i, j, k] : a)                     // { dg-error "use of 'i' before deduction of 
'auto'" "" { target *-*-* } .-1 }
+  for (auto [i, j, k] : a)                     // { dg-error "use of 'i' before 
deduction of 'auto'" }
      for (int l = i; l < j; l += k)         // { dg-error "use of 'j' before 
deduction of 'auto'" }
-      ;                                                // { dg-error "use of 'k' before 
deduction of 'auto'" "" { target *-*-* } .-3 }
+      ;                                                // { dg-error "use of 'k' before 
deduction of 'auto'" "" { target *-*-* } .-1 }

Hmm, this error is surprising: since the initializer is non-dependent, we should have deduced immediately. I'd expect the same error as in the non-structured-binding cases, "* expression refers to iteration variable".

  }
template <typename T>
--- gcc/testsuite/g++.dg/gomp/for-22.C.jj       2022-11-29 13:07:10.859237506 
+0100
+++ gcc/testsuite/g++.dg/gomp/for-22.C  2022-11-29 13:09:50.743897003 +0100
@@ -0,0 +1,57 @@
+// { dg-do compile { target c++17 } }
+
+namespace std {
+  template<typename T> struct tuple_size;
+  template<int, typename> struct tuple_element;
+}
+
+struct A {
+  int i;
+  template <int I> int& get() { return i; }
+};
+
+template<> struct std::tuple_size<A> { static const int value = 3; };
+template<int I> struct std::tuple_element<I,A> { using type = int; };
+
+struct B {
+  A *begin();
+  A *end();
+};
+
+void
+f1 (B a)
+{
+  #pragma omp for collapse (2)
+  for (auto [i, j, k] : a)                     // { dg-error "use of 'i' before deduction of 
'auto'" "" { target *-*-* } .+1 }
+    for (int l = i; l < j; l += k)          // { dg-error "use of 'j' before 
deduction of 'auto'" }
+      ;                                                // { dg-error "use of 'k' before 
deduction of 'auto'" "" { target *-*-* } .-1 }
+}
+
+template <int N>
+void
+f2 (B a)
+{
+  #pragma omp for collapse (2)
+  for (auto [i, j, k] : a)                     // { dg-error "use of 'i' before deduction of 
'auto'" "" { target *-*-* } .+1 }
+    for (int l = i; l < j; l += k)          // { dg-error "use of 'j' before 
deduction of 'auto'" }
+      ;                                                // { dg-error "use of 'k' before 
deduction of 'auto'" "" { target *-*-* } .-1 }
+}
+
+template <typename T>
+void
+f3 (T a)
+{
+  #pragma omp for collapse (2)
+  for (auto [i, j, k] : a)                     // { dg-error "use of 'i' before deduction of 
'auto'" "" { target *-*-* } .-1 }
+    for (int l = i; l < j; l += k)          // { dg-error "use of 'j' before 
deduction of 'auto'" }
+      ;                                                // { dg-error "use of 'k' before 
deduction of 'auto'" "" { target *-*-* } .-3 }
+}
+
+void
+test ()
+{
+  B b;
+  f1 (b);
+  f2 <0> (b);
+  f3 <B> (b);
+}

        Jakub


Reply via email to