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?
>
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk and perhaps 11?
> >
> > PR c++/99895
> >
> > gcc/cp/ChangeLog:
> >
> > * call.cc (build_over_call): Don't evaluate non-dependent
> > immediate member function calls here.
> > (build_new_method_call): Instead evaluate them here.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/cpp2a/consteval-memfn1.C: New test.
> > * g++.dg/cpp2a/consteval-memfn2.C: New test.
> > ---
> > gcc/cp/call.cc | 9 ++++-
> > gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C | 15 ++++++++
> > gcc/testsuite/g++.dg/cpp2a/consteval-memfn2.C | 34 +++++++++++++++++++
> > 3 files changed, 57 insertions(+), 1 deletion(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval-memfn2.C
> >
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index d4a07a7a9b3..0583cc0083b 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -9241,7 +9241,10 @@ build_over_call (struct z_candidate *cand, int
> > flags, tsubst_flags_t complain)
> > addr, nargs, argarray);
> > if (TREE_THIS_VOLATILE (fn) && cfun)
> > current_function_returns_abnormally = 1;
> > - if (immediate_invocation_p (fn, nargs))
> > + if (!DECL_FUNCTION_MEMBER_P (fn)
> > + /* Non-dependent immediate member function calls are evaluated in
> > + build_new_method_call. */
> > + && immediate_invocation_p (fn, nargs))
> > {
> > tree obj_arg = NULL_TREE, exprimm = expr;
> > if (DECL_CONSTRUCTOR_P (fn))
> > @@ -11227,6 +11230,10 @@ skip_prune:
> > call = convert_from_reference (call);
> > if (cast_to_void)
> > call = build_nop (void_type_node, call);
> > +
> > + if (immediate_invocation_p (fn, vec_safe_length (orig_args)))
> > + fold_non_dependent_expr (call, complain,
> > + /*manifestly_const_eval=*/true);
> > }
> >
> > /* Free all the conversions we allocated. */
> > 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..d2df2e9b5ae
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/consteval-memfn1.C
> > @@ -0,0 +1,15 @@
> > +// 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;
> > + }
> > +};
> > +
> > +template<class>
> > +void VerifyHash(fixed_string s) {
> > + s.size(0); // { dg-bogus "" }
> > + s.size(-1); // { dg-message "expansion of" }
> > +}
> > 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">();
> > +}
>
