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...
>
> >
> > > 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">();
> > > +}
> >
>
