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

Patrick Palka ppalka@redhat.com
Wed Jan 26 22:46:49 GMT 2022


On Wed, 26 Jan 2022, Patrick Palka wrote:

> On Wed, Jan 19, 2022 at 2:19 PM Jason Merrill <jason@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">();
> > > +}
> >
> 


More information about the Gcc-patches mailing list