On 1/27/22 09:02, Patrick Palka wrote:
On Wed, 26 Jan 2022, Patrick Palka wrote:

On Wed, 26 Jan 2022, Patrick Palka wrote:

On Wed, Jan 19, 2022 at 2:19 PM Jason Merrill <ja...@redhat.com> wrote:

On 1/19/22 11:15, Patrick Palka wrote:
Here we're emitting a bogus error during ahead of time evaluation of a
non-dependent immediate member function call such as a.f(args) because
the defacto templated form for such a call is (a.f)(args) but we're
trying to evaluate it using the intermediate CALL_EXPR built by
build_over_call, which has the non-member form f(a, args).  The defacto
member form is built in build_new_method_call, so it seems we should
handle the immediate call there instead.

Hmm, there's already a bunch of code in build_over_call to try to fix up
the object argument, and there seem to be many places other than
build_new_method_call that call build_over_call for member functions; I
think it's probably better to build the needed COMPONENT_REF in
build_over_call.

Ah, but in build_over_call the arguments (including the implicit
object argument) are potentially wrapped in a NON_DEPENDENT_EXPR.  So
even if we built a COMPONENT_REF in build_over_call, we'd  have to
keep rebuilding the entire CALL_EXPR (including the COMPONENT_REF) in
terms of the original arguments in build_new_method_call, IIUC.

On a related note, the NON_DEPENDENT_EXPR wrapping seems to be a
problem for non-member functions too, because the constexpr engine
punts on them:

   struct fixed_string { };

   static consteval void size(fixed_string) { }

   template<class>
   void VerifyHash(fixed_string s) {
     size(s);  // error: expression ā€˜sā€™ is not a constant expression
(because it's wrapped in NON_DEPENDENT_EXPR)
   }

I wonder if this means we should be evaluating non-dependent
non-member immediate calls elsewhere, e.g. in finish_call_expr?  Or
perhaps we should stop punting on NON_DEPENDENT_EXPR during constexpr
evaluation?

Actually, for that particular example, we probably should just avoid
wrapping PARM_DECL in a NON_DEPENDENT_EXPR...

Here's a patch that makes build_over_call build the COMPONENT_REF-using
form for non-dependent member calls, and in passing makes us avoid
wrapping PARM_DECL in a NON_DEPENDENT_EXPR.

-- >8 --

Subject: [PATCH] c++: non-dependent immediate member fn call [PR99895]

Here we're emitting a bogus error during ahead of time evaluation of a
non-dependent immediate member function call such as a.f(args) because
the defacto templated form for such a call is (a.f)(args) but we're
trying to evaluate it using the intermediate CALL_EXPR built by
build_over_call, which has the non-member form f(a, args).  The defacto
member form is built in build_new_method_call, so it seems we should
handle the immediate call there instead, or perhaps make build_over_call
build the correct form in the first place.

Giiven that there are many spots other than build_new_method_call that
call build_over_call for member functions, this patch takes the latter
approach.

In passing, this patch makes us avoid wrapping PARM_DECL in
NON_DEPENDENT_EXPR for benefit of the third testcase below.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

OK.

        PR c++/99895

gcc/cp/ChangeLog:

        * call.cc (build_over_call): For a non-dependent member call,
        build up a CALL_EXPR using a COMPONENT_REF callee, as in
        build_new_method_call.
        * pt.cc (build_non_dependent_expr): Don't wrap PARM_DECL either.
        * tree.cc (build_min_non_dep_op_overload): Adjust accordingly
        after the build_over_call change.

gcc/ChangeLog:

        * tree.cc (build_call_vec): Add const to second parameter.
        * tree.h (build_call_vec): Likewise.

gcc/testsuite/ChangeLog:

        * g++.dg/cpp2a/consteval-memfn1.C: New test.
        * g++.dg/cpp2a/consteval-memfn2.C: New test.
        * g++.dg/cpp2a/consteval28.C: New test.
---
  gcc/cp/call.cc                                | 38 +++++++------------
  gcc/cp/pt.cc                                  |  6 ++-
  gcc/cp/tree.cc                                |  5 ++-
  gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C | 27 +++++++++++++
  gcc/testsuite/g++.dg/cpp2a/consteval-memfn2.C | 34 +++++++++++++++++
  gcc/testsuite/g++.dg/cpp2a/consteval28.C      | 10 +++++
  gcc/tree.cc                                   |  2 +-
  gcc/tree.h                                    |  2 +-
  8 files changed, 94 insertions(+), 30 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval-memfn2.C
  create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval28.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index bc157cdd1fb..b2e89c5d783 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9204,11 +9204,6 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
       errors will be deferred until the template is instantiated.  */
    if (processing_template_decl)
      {
-      tree expr, addr;
-      tree return_type;
-      const tree *argarray;
-      unsigned int nargs;
-
        if (undeduced_auto_decl (fn))
        mark_used (fn, complain);
        else
@@ -9216,32 +9211,27 @@ build_over_call (struct z_candidate *cand, int flags, 
tsubst_flags_t complain)
           See PR80598.  */
        TREE_USED (fn) = 1;
- return_type = TREE_TYPE (TREE_TYPE (fn));
-      nargs = vec_safe_length (args);
+      tree return_type = TREE_TYPE (TREE_TYPE (fn));
+      tree callee;
        if (first_arg == NULL_TREE)
-       argarray = args->address ();
+       {
+         callee = build_addr_func (fn, complain);
+         if (callee == error_mark_node)
+           return error_mark_node;
+       }
        else
        {
-         tree *alcarray;
-         unsigned int ix;
-         tree arg;
-
-         ++nargs;
-         alcarray = XALLOCAVEC (tree, nargs);
-         alcarray[0] = build_this (first_arg);
-         FOR_EACH_VEC_SAFE_ELT (args, ix, arg)
-           alcarray[ix + 1] = arg;
-         argarray = alcarray;
+         tree binfo = TYPE_BINFO (TREE_TYPE (first_arg));
+         callee = build_baselink (binfo, binfo, fn, NULL_TREE);
+         callee = build_min (COMPONENT_REF, TREE_TYPE (fn),
+                             first_arg, callee, NULL_TREE);
        }
- addr = build_addr_func (fn, complain);
-      if (addr == error_mark_node)
-       return error_mark_node;
-      expr = build_call_array_loc (input_location, return_type,
-                                  addr, nargs, argarray);
+      tree expr = build_call_vec (return_type, callee, args);
+      SET_EXPR_LOCATION (expr, input_location);
        if (TREE_THIS_VOLATILE (fn) && cfun)
        current_function_returns_abnormally = 1;
-      if (immediate_invocation_p (fn, nargs))
+      if (immediate_invocation_p (fn, vec_safe_length (args)))
        {
          tree obj_arg = NULL_TREE, exprimm = expr;
          if (DECL_CONSTRUCTOR_P (fn))
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 6fbda676527..19e73b3b77d 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -28410,8 +28410,10 @@ build_non_dependent_expr (tree expr)
    if (is_overloaded_fn (inner_expr)
        || TREE_CODE (inner_expr) == OFFSET_REF)
      return orig_expr;
-  /* There is no need to return a proxy for a variable or enumerator.  */
-  if (VAR_P (expr) || TREE_CODE (expr) == CONST_DECL)
+  /* There is no need to return a proxy for a variable, parameter
+     or enumerator.  */
+  if (VAR_P (expr) || TREE_CODE (expr) == PARM_DECL
+      || TREE_CODE (expr) == CONST_DECL)
      return orig_expr;
    /* Preserve string constants; conversions from string constants to
       "char *" are allowed, even though normally a "const char *"
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 5d453e49717..056f10f13b4 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -3661,6 +3661,8 @@ build_min_non_dep_op_overload (enum tree_code op,
    nargs = call_expr_nargs (non_dep);
expected_nargs = cp_tree_code_length (op);
+  if (TREE_CODE (TREE_TYPE (overload)) == METHOD_TYPE)
+    expected_nargs -= 1;
    if ((op == POSTINCREMENT_EXPR
         || op == POSTDECREMENT_EXPR)
        /* With -fpermissive non_dep could be operator++().  */
@@ -3687,7 +3689,7 @@ build_min_non_dep_op_overload (enum tree_code op,
        tree method = build_baselink (binfo, binfo, overload, NULL_TREE);
        fn = build_min (COMPONENT_REF, TREE_TYPE (overload),
                      object, method, NULL_TREE);
-      for (int i = 1; i < nargs; i++)
+      for (int i = 0; i < nargs; i++)
        {
          tree arg = va_arg (p, tree);
          vec_safe_push (args, arg);
@@ -3723,7 +3725,6 @@ build_min_non_dep_op_overload (tree non_dep, tree 
overload, tree object,
    tree method = build_baselink (binfo, binfo, overload, NULL_TREE);
    tree fn = build_min (COMPONENT_REF, TREE_TYPE (overload),
                       object, method, NULL_TREE);
-  nargs--;
    gcc_assert (vec_safe_length (args) == nargs);
tree call = build_min_non_dep_call_vec (non_dep, fn, args);
diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C 
b/gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C
new file mode 100644
index 00000000000..910e7a1ac1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C
@@ -0,0 +1,27 @@
+// PR c++/99895
+// { dg-do compile { target c++20 } }
+
+struct fixed_string {
+  consteval int size(int n) const {
+    if (n < 0) throw; // { dg-error "not a constant" }
+    return n;
+  }
+
+  static consteval int size_static(int n) {
+    if (n < 0) throw; // { dg-error "not a constant" }
+    return n;
+  }
+
+  consteval void operator()() const { }
+};
+
+template<class>
+void VerifyHash(fixed_string s) {
+  s.size(0); // { dg-bogus "" }
+  s.size(-1); // { dg-message "expansion of" }
+  s.size_static(0); // { dg-bogus "" }
+  s.size_static(-1); // { dg-message "expansion of" }
+  fixed_string::size_static(0); // { dg-bogus "" }
+  fixed_string::size_static(-1); // { dg-message "expansion of" }
+  s(); // { dg-bogus "" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval-memfn2.C 
b/gcc/testsuite/g++.dg/cpp2a/consteval-memfn2.C
new file mode 100644
index 00000000000..71748f46b13
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval-memfn2.C
@@ -0,0 +1,34 @@
+// PR c++/99895
+// { dg-do compile { target c++20 } }
+
+static constexpr unsigned hash(const char* s, unsigned length)
+{
+    s=s;
+    return length;
+}
+template<unsigned N>
+struct fixed_string
+{
+    constexpr fixed_string(const char (&s)[N])
+    {
+        for (int i = 0; i < N; i++)
+            str[i] = s[i];
+    }
+    consteval const char* data() const { return str; }
+    consteval unsigned size() const { return N-1; }
+    char str[N];
+};
+template<unsigned expected_hash, fixed_string... s>
+static consteval void VerifyHash()
+{
+    (
+      [](auto){static_assert(hash(s.data(), s.size()) == expected_hash);}(s)
+    ,...);
+    // The compiler mistakenly translates s.data() into s.data(&s)
+    // and then complains that the call is not valid, because
+    // the function expects 0 parameters and 1 "was provided".
+}
+void foo()
+{
+    VerifyHash<5, "khaki", "plums">();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval28.C 
b/gcc/testsuite/g++.dg/cpp2a/consteval28.C
new file mode 100644
index 00000000000..293a6be2fc5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval28.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++20 } }
+
+struct empty { };
+
+consteval void f(empty) { }
+
+template<class>
+void g(empty e) {
+  f(e);
+}
diff --git a/gcc/tree.cc b/gcc/tree.cc
index c4b36619e6a..9d445b2740f 100644
--- a/gcc/tree.cc
+++ b/gcc/tree.cc
@@ -10492,7 +10492,7 @@ build_call_array_loc (location_t loc, tree return_type, 
tree fn,
  /* Like build_call_array, but takes a vec.  */
tree
-build_call_vec (tree return_type, tree fn, vec<tree, va_gc> *args)
+build_call_vec (tree return_type, tree fn, const vec<tree, va_gc> *args)
  {
    tree ret, t;
    unsigned int ix;
diff --git a/gcc/tree.h b/gcc/tree.h
index 30bc53c2996..4c01d94244e 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -4589,7 +4589,7 @@ extern tree build_call_valist (tree, tree, int, va_list);
  #define build_call_array(T1,T2,N,T3)\
     build_call_array_loc (UNKNOWN_LOCATION, T1, T2, N, T3)
  extern tree build_call_array_loc (location_t, tree, tree, int, const tree *);
-extern tree build_call_vec (tree, tree, vec<tree, va_gc> *);
+extern tree build_call_vec (tree, tree, const vec<tree, va_gc> *);
  extern tree build_call_expr_loc_array (location_t, tree, int, tree *);
  extern tree build_call_expr_loc_vec (location_t, tree, vec<tree, va_gc> *);
  extern tree build_call_expr_loc (location_t, tree, int, ...);

Reply via email to