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?
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, ...);
--
2.35.0
