Hi Jason,
On 30 Sep 2024, at 19:56, Jason Merrill wrote:
> On 9/23/24 4:44 AM, Simon Martin wrote:
>> Hi Jason,
>>
>> On 20 Sep 2024, at 18:01, Jason Merrill wrote:
>>
>>> On 9/20/24 5:21 PM, Simon Martin wrote:
>>>> The following code triggers an ICE
>>>>
>>>> === cut here ===
>>>> class base {};
>>>> class derived : virtual public base {
>>>> public:
>>>> template<typename Arg> constexpr derived(Arg) {}
>>>> };
>>>> int main() {
>>>> derived obj(1.);
>>>> }
>>>> === cut here ===
>>>>
>>>> The problem is that cxx_bind_parameters_in_call ends up attempting
>>>> to
>>
>>>> convert a REAL_CST (the first non artificial parameter) to
>>>> INTEGER_TYPE
>>>> (the type of the __in_chrg parameter), which ICEs.
>>>>
>>>> This patch teaches cxx_bind_parameters_in_call to handle the
>>>> __in_chrg
>>>> and __vtt_parm parameters that {con,de}structors might have.
>>>>
>>>> Note that in the test case, the constructor is not
>>>> constexpr-suitable,
>>>> however it's OK since it's a template according to my read of
>>>> paragraph
>>>> (3) of [dcl.constexpr].
>>>
>>> Agreed.
>>>
>>> It looks like your patch doesn't correct the mismatching of
>>> arguments
>>> to parameters that you describe, but at least for now it should be
>>> enough to set *non_constant_p and return if we see a VTT or
>>> in-charge
>>> parameter.
>>>
>> Thanks, it’s true that my initial patch was wrong in that we’d
>> leave
>> cxx_bind_parameters_in_call thinking the expression was actually a
>> constant expression :-/
>>
>> The attached revised patch follows your suggestion (thanks!).
>> Successfully tested on x86_64-pc-linux-gnu. OK for trunk?
>
> After this patch I'm seeing a regression on constexpr-dynamic10.C with
> -fimplicit-constexpr; we also need to give an error here when
> (!ctx->quiet).
Thanks, good catch. TIL about --stds=impcx...
The attached patch fixes the issue, and was successfully tested on
x86_64-pc-linux-gnu, including with “make -C gcc -k check-c++-all”.
OK for trunk?
Note that it includes a new test that’s basically a copy of
constexpr-dynamic10.C, with -fimplicit-constexpr. Is that the right
thing to do or should I just leave the test suite as is, knowing that
someone/something will run the test suite with --stds=impcx at some
point before a release?
And the next natural question is whether I should have also tested with
--stds=impcx before my initial submission?
https://gcc.gnu.org/contribute.html#testing advises to test with “make
-k check”; maybe we need to also mention check-c++-all for changes to
the C++ front-end?
Thanks again, Simon
From c724f939881c6808d2fc36b63e0093ec59af3a40 Mon Sep 17 00:00:00 2001
From: Simon Martin <si...@nasilyan.com>
Date: Tue, 1 Oct 2024 11:07:51 +0200
Subject: [PATCH] c++: Fix regression introduced by r15-3796 [PR116722]
Jason pointed out that the fix I made for PR116722 via r15-3796
introduces a regression when running constexpr-dynamic10.C with
-fimplicit-constexpr.
The problem is that my change makes us leave cxx_eval_call_expression
early, and bypass the call to cxx_eval_thunk_call (through a recursive
call to cxx_eval_call_expression) that used to emit an error for that
testcase with -fimplicit-constexpr.
This patch emits the error if !ctx->quiet before bailing out because the
{con,de}structor belongs to a class with virtual bases.
Successfully tested on x86_64-pc-linux-gnu and also with --stds=impcx,
that showed a failure and does not anymore.
PR c++/116722
gcc/cp/ChangeLog:
* constexpr.cc (cxx_bind_parameters_in_call): When !ctx->quiet,
emit error before bailing out due to a call to {con,de}structor
for a class with virtual bases.
gcc/testsuite/ChangeLog:
* g++.dg/ext/fimplicit-constexpr2.C: New test.
---
gcc/cp/constexpr.cc | 5 +++++
gcc/testsuite/g++.dg/ext/fimplicit-constexpr2.C | 14 ++++++++++++++
2 files changed, 19 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/ext/fimplicit-constexpr2.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 5c6696740fc..36ad894cefb 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1867,6 +1867,11 @@ cxx_bind_parameters_in_call (const constexpr_ctx *ctx,
tree t, tree fun,
with virtual bases. */
if (DECL_HAS_IN_CHARGE_PARM_P (fun) || DECL_HAS_VTT_PARM_P (fun))
{
+ if (!ctx->quiet
+ && constexpr_error (cp_expr_loc_or_input_loc (t),
+ /*constexpr_fundef_p=*/false,
+ "call to non-%<constexpr%> function %qD", fun))
+ explain_invalid_constexpr_fn (fun);
*non_constant_p = true;
return binds;
}
diff --git a/gcc/testsuite/g++.dg/ext/fimplicit-constexpr2.C
b/gcc/testsuite/g++.dg/ext/fimplicit-constexpr2.C
new file mode 100644
index 00000000000..76c050973a9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/fimplicit-constexpr2.C
@@ -0,0 +1,14 @@
+// PR c++/116722
+// Same as constexpr-dynamic10.C, but with -fimplicit-constexpr
+// { dg-do compile { target c++20 } }
+// { dg-additional-options -fimplicit-constexpr }
+
+// Virtual base.
+
+struct C { virtual void a(); };
+struct B { virtual void b(); };
+struct A : virtual B, C { virtual void c(); }; // { dg-error ".struct A. has
virtual base classes" }
+
+constexpr A a; // { dg-error "call" }
+
+constexpr bool b1 = (dynamic_cast<C&>((B&)a), false);
--
2.44.0
