On Mon, Sep 17, 2018 at 5:39 PM, Marek Polacek <pola...@redhat.com> wrote:
> On Fri, Sep 14, 2018 at 04:45:22PM -0400, Marek Polacek wrote:
>> On Fri, Sep 14, 2018 at 04:30:46PM -0400, Jason Merrill wrote:
>> > On Fri, Sep 14, 2018 at 1:19 PM, Marek Polacek <pola...@redhat.com> wrote:
>> > > This patch implements another bit of C++20, virtual calls in constant
>> > > expression:
>> > > <http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1064r0.html>
>> > > The basic idea is that since in a constant expression we know the dynamic
>> > > type (to detect invalid code etc.), the restriction that prohibits
>> > > virtual
>> > > calls is unnecessary.
>> > >
>> > > Handling virtual function calls turned out to be fairly easy (as
>> > > anticipated);
>> > > I simply let the constexpr machinery figure out the dynamic type and then
>> > > OBJ_TYPE_REF_TOKEN gives us the index into the virtual function table.
>> > > That
>> > > way we get the function decl we're interested in, and
>> > > cxx_eval_call_expression
>> > > takes it from there.
>> > >
>> > > But handling pointer-to-virtual-member-functions doesn't work like that.
>> > > get_member_function_from_ptrfunc creates a COND_EXPR which looks like
>> > > if (pf.__pfn & 1) // is it a virtual function?
>> > > // yes, find the pointer in the vtable
>> > > else
>> > > // no, just return the pointer
>> > > so ideally we want to evaluate the then-branch. Eventually it'll
>> > > evaluate it
>> > > to something like _ZTV2X2[2], but the vtable isn't constexpr so we'd end
>> > > up
>> > > with "not a constant expression" error.
>> >
>> > Then let's mark the vtable as constexpr, there's no reason for it not to
>> > be.
>
> Done. But then I had to add indexes to the vtable's ctor (because
> find_array_ctor_elt
> expects it), which broke an assert in gimple_get_virt_method_for_vtable. But
> I
> don't need the array_ref hack anymore!
> Also, I had to set DECL_DECLARED_CONSTEXPR_P after maybe_commonize_var,
> otherwise we run into the sorry in that function with -fno-weak...
Hmm, we shouldn't give that sorry for DECL_ARTIFICIAL variables.
Looking more closely, it seems that the call to maybe_commonize_var
from initialize_artificial_var did nothing before this change, since
the vtable is DECL_ARTIFICIAL, so it didn't pass the condition at the
top. I suppose we should extend the !DECL_ARTIFICIAL check in
maybe_commonize_var to the inline variable case as well.
>> Ok, unfortunately it wasn't as easy as merely marking it
>> DECL_DECLARED_CONSTEXPR_P
>> in initialize_artificial_var because then I saw "used in its own initializer"
>> error. Which I don't know why, but now that I know you agree with this
>> direction
>> I can dig deeper.
>>
>> > > Since the vtable initializer is
>> > > a compile-time constant, I thought we could make it work by a hack as
>> > > the one
>> > > in cxx_eval_array_reference. We'll then let cxx_eval_call_expression do
>> > > its
>> > > job and everything is hunky-dory.
>> > >
>> > > Except when it isn't: I noticed that the presence of _vptr doesn't make
>> > > the
>> > > class non-empty, and when cxx_eval_constant_expression saw a decl with
>> > > an empty
>> > > class type, it just evaluated it to { }. But such a class still had
>> > > gotten an
>> > > initializer that looks like {.D.2082 = {._vptr.X2 = &_ZTV2X2 + 16}}. So
>> > > replacing it with { } will lose the proper initializer whereupon we fail.
>> > > The check I've added to cxx_eval_constant_expression won't win any beauty
>> > > contests but unfortunately EMPTY_CONSTRUCTOR_P doesn't work there.
>> >
>> > Perhaps we should check !TYPE_POLYMORPHIC_P as well as
>> > is_really_empty_class. Perhaps there should be a predicate for that,
>> > say, is_really_nearly_empty_class...
>
> For now I've only added the !TYPE_POLYMORPHIC_P check, which works just fine.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-09-17 Marek Polacek <pola...@redhat.com>
>
> P1064R0 - Allowing Virtual Function Calls in Constant Expressions
> * call.c (build_over_call): Add FIXME.
> * class.c (initialize_vtable): Mark the vtable as constexpr.
> (build_vtbl_initializer): Build vtable's constructor with indexes.
> * constexpr.c (cxx_eval_constant_expression): Don't ignore _vptr's
> initializer. Handle OBJ_TYPE_REF.
> (potential_constant_expression_1): Handle OBJ_TYPE_REF.
> * decl.c (grokdeclarator): Change error to pedwarn. Only warn when
> pedantic and not C++2a.
>
> * gimple-fold.c (gimple_get_virt_method_for_vtable): Remove assert.
>
> * g++.dg/cpp0x/constexpr-virtual5.C: Adjust dg-error.
> * g++.dg/cpp2a/constexpr-virtual1.C: New test.
> * g++.dg/cpp2a/constexpr-virtual2.C: New test.
> * g++.dg/cpp2a/constexpr-virtual3.C: New test.
> * g++.dg/cpp2a/constexpr-virtual4.C: New test.
> * g++.dg/cpp2a/constexpr-virtual5.C: New test.
> * g++.dg/cpp2a/constexpr-virtual6.C: New test.
> * g++.dg/cpp2a/constexpr-virtual7.C: New test.
> * g++.dg/cpp2a/constexpr-virtual8.C: New test.
> * g++.dg/cpp2a/constexpr-virtual9.C: New test.
> * g++.dg/diagnostic/virtual-constexpr.C: Skip for C++2a. Use
> -pedantic-errors. Adjust dg-error.
>
> diff --git gcc/cp/call.c gcc/cp/call.c
> index 69503ca7920..6c70874af40 100644
> --- gcc/cp/call.c
> +++ gcc/cp/call.c
> @@ -8401,7 +8401,8 @@ build_over_call (struct z_candidate *cand, int flags,
> tsubst_flags_t complain)
>
> if (DECL_VINDEX (fn) && (flags & LOOKUP_NONVIRTUAL) == 0
> /* Don't mess with virtual lookup in instantiate_non_dependent_expr;
> - virtual functions can't be constexpr. */
> + virtual functions can't be constexpr. FIXME Actually, no longer
> + true in C++2a. */
> && !in_template_function ())
I notice that removing the in_template_function check doesn't break
template/virtual4.C nowadays. Does it break anything else?
> + /* The C++ FE now produces indexed fields but we can index the array
> + directly. */
> if (access_index < CONSTRUCTOR_NELTS (init))
> {
> fn = CONSTRUCTOR_ELT (init, access_index)->value;
> - gcc_checking_assert (!CONSTRUCTOR_ELT (init, access_index)->index);
Rather than remove this assert, let's fix it to check that the ->index
matches access_index.
Jason
