* [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?:
@ 2019-05-07 20:43 Jason Merrill
2019-05-22 19:46 ` Jason Merrill
2019-05-22 22:00 ` Jason Merrill
0 siblings, 2 replies; 5+ messages in thread
From: Jason Merrill @ 2019-05-07 20:43 UTC (permalink / raw)
To: gcc-patches
* typeck.c (build_static_cast_1): Use cp_build_addr_expr.
For GCC 9 I fixed this bug with a patch to gimplify_cond_expr, but this
function was also doing the wrong thing.
Using build_address does not push the ADDR_EXPR down into the arms of a
COND_EXPR, which we need for proper handling of conversion of an lvalue ?:
to another reference type.
---
gcc/cp/typeck.c | 5 +++--
gcc/cp/ChangeLog | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index c107a321949..f039a3b3eb0 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -5916,7 +5916,8 @@ condition_conversion (tree expr)
}
/* Returns the address of T. This function will fold away
- ADDR_EXPR of INDIRECT_REF. */
+ ADDR_EXPR of INDIRECT_REF. This is only for low-level usage;
+ most places should use cp_build_addr_expr instead. */
tree
build_address (tree t)
@@ -7114,7 +7115,7 @@ build_static_cast_1 (tree type, tree expr, bool c_cast_p,
base = lookup_base (TREE_TYPE (type), intype,
c_cast_p ? ba_unique : ba_check,
NULL, complain);
- expr = build_address (expr);
+ expr = cp_build_addr_expr (expr, complain);
if (sanitize_flags_p (SANITIZE_VPTR))
{
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index bd0914b8ffa..d90cc099767 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,5 +1,8 @@
2019-05-07 Jason Merrill <jason@redhat.com>
+ PR c++/86485 - -Wmaybe-unused with empty class ?:
+ * typeck.c (build_static_cast_1): Use cp_build_addr_expr.
+
* pt.c (type_dependent_expression_p): A non-type template parm with
a placeholder type is type-dependent.
base-commit: 1f51079362f27895c9c4e125549f6cc1b4d50568
prerequisite-patch-id: f9e234ae0c68beb8fed9e212e3da578a940c2995
--
2.20.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?:
2019-05-07 20:43 [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?: Jason Merrill
@ 2019-05-22 19:46 ` Jason Merrill
2019-05-22 22:00 ` Jason Merrill
1 sibling, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2019-05-22 19:46 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 780 bytes --]
On Tue, May 7, 2019 at 4:43 PM Jason Merrill <jason@redhat.com> wrote:
>
> * typeck.c (build_static_cast_1): Use cp_build_addr_expr.
>
> For GCC 9 I fixed this bug with a patch to gimplify_cond_expr, but this
> function was also doing the wrong thing.
>
> Using build_address does not push the ADDR_EXPR down into the arms of a
> COND_EXPR, which we need for proper handling of conversion of an lvalue ?:
> to another reference type.
Yet another tweak that would have fixed this bug: we should treat INIT_EXPR
and MODIFY_EXPR differently for determining whether this is a simple empty
class copy, since a TARGET_EXPR on the RHS is direct initialization if
INIT_EXPR but copy if MODIFY_EXPR.
* cp-gimplify.c (simple_empty_class_p): Also true for MODIFY_EXPR.
[-- Attachment #2: 86485-simple.txt --]
[-- Type: text/plain, Size: 2097 bytes --]
commit 4ee9e054cba31bd532061bd357477e757b5b284a
Author: Jason Merrill <jason@redhat.com>
Date: Sat Mar 2 01:07:41 2019 -0500
PR c++/86485 - simple_empty_class_p
Yet another tweak that would have fixed this bug: we should treat INIT_EXPR
and MODIFY_EXPR differently for determining whether this is a simple empty
class copy, since a TARGET_EXPR on the RHS is direct initialization if
INIT_EXPR but copy if MODIFY_EXPR.
* cp-gimplify.c (simple_empty_class_p): Also true for MODIFY_EXPR.
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index e8c22c071c2..30937b1a1a3 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -594,19 +594,20 @@ gimplify_must_not_throw_expr (tree *expr_p, gimple_seq *pre_p)
return slot optimization alone because it isn't a copy. */
static bool
-simple_empty_class_p (tree type, tree op)
+simple_empty_class_p (tree type, tree op, tree_code code)
{
+ if (TREE_CODE (op) == COMPOUND_EXPR)
+ return simple_empty_class_p (type, TREE_OPERAND (op, 1), code);
return
- ((TREE_CODE (op) == COMPOUND_EXPR
- && simple_empty_class_p (type, TREE_OPERAND (op, 1)))
- || TREE_CODE (op) == EMPTY_CLASS_EXPR
+ (TREE_CODE (op) == EMPTY_CLASS_EXPR
+ || code == MODIFY_EXPR
|| is_gimple_lvalue (op)
|| INDIRECT_REF_P (op)
|| (TREE_CODE (op) == CONSTRUCTOR
- && CONSTRUCTOR_NELTS (op) == 0
- && !TREE_CLOBBER_P (op))
+ && CONSTRUCTOR_NELTS (op) == 0)
|| (TREE_CODE (op) == CALL_EXPR
&& !CALL_EXPR_RETURN_SLOT_OPT (op)))
+ && !TREE_CLOBBER_P (op)
&& is_really_empty_class (type, /*ignore_vptr*/true);
}
@@ -715,7 +716,7 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
TREE_OPERAND (*expr_p, 1) = build1 (VIEW_CONVERT_EXPR,
TREE_TYPE (op0), op1);
- else if (simple_empty_class_p (TREE_TYPE (op0), op1))
+ else if (simple_empty_class_p (TREE_TYPE (op0), op1, code))
{
/* Remove any copies of empty classes. Also drop volatile
variables on the RHS to avoid infinite recursion from
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?:
2019-05-07 20:43 [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?: Jason Merrill
2019-05-22 19:46 ` Jason Merrill
@ 2019-05-22 22:00 ` Jason Merrill
2019-05-24 14:13 ` Jakub Jelinek
1 sibling, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2019-05-22 22:00 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 590 bytes --]
On Tue, May 7, 2019 at 4:43 PM Jason Merrill <jason@redhat.com> wrote:
>
> * typeck.c (build_static_cast_1): Use cp_build_addr_expr.
>
> For GCC 9 I fixed this bug with a patch to gimplify_cond_expr, but this
> function was also doing the wrong thing.
>
> Using build_address does not push the ADDR_EXPR down into the arms of a
> COND_EXPR, which we need for proper handling of conversion of an lvalue ?:
> to another reference type.
And that allows the gimplifier to assert that we should never see a
COND_EXPR of addressable type.
Tested x86_64-pc-linux-gnu, applying to trunk.
[-- Attachment #2: cond-addr.txt --]
[-- Type: text/plain, Size: 1157 bytes --]
commit c4f1e37204aaea7efb2aa7dc234d5c8ebeba1089
Author: Jason Merrill <jason@redhat.com>
Date: Mon Mar 4 14:09:57 2019 -0500
* gimplify.c (gimplify_cond_expr): Don't check TREE_ADDRESSABLE.
The front end shouldn't produce a GENERIC COND_EXPR of TREE_ADDRESSABLE
type.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 5bacb255ba7..6905165ad33 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3990,10 +3990,12 @@ gimplify_cond_expr (tree *expr_p, gimple_seq *pre_p, fallback_t fallback)
tree result;
/* If either an rvalue is ok or we do not require an lvalue, create the
- temporary. But we cannot do that if the type is addressable. */
+ temporary. We cannot do that if the type is addressable, but
+ that should have been avoided before we got here. */
if (((fallback & fb_rvalue) || !(fallback & fb_lvalue))
- && !TREE_ADDRESSABLE (type))
+ && (flag_checking || !TREE_ADDRESSABLE (type)))
{
+ gcc_assert (!TREE_ADDRESSABLE (type));
if (gimplify_ctxp->allow_rhs_cond_expr
/* If either branch has side effects or could trap, it can't be
evaluated unconditionally. */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?:
2019-05-22 22:00 ` Jason Merrill
@ 2019-05-24 14:13 ` Jakub Jelinek
2019-05-24 14:39 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Jelinek @ 2019-05-24 14:13 UTC (permalink / raw)
To: Jason Merrill, Iain Buclaw; +Cc: gcc-patches List
On Wed, May 22, 2019 at 05:59:56PM -0400, Jason Merrill wrote:
> On Tue, May 7, 2019 at 4:43 PM Jason Merrill <jason@redhat.com> wrote:
> >
> > * typeck.c (build_static_cast_1): Use cp_build_addr_expr.
> >
> > For GCC 9 I fixed this bug with a patch to gimplify_cond_expr, but this
> > function was also doing the wrong thing.
> >
> > Using build_address does not push the ADDR_EXPR down into the arms of a
> > COND_EXPR, which we need for proper handling of conversion of an lvalue ?:
> > to another reference type.
>
> And that allows the gimplifier to assert that we should never see a
> COND_EXPR of addressable type.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
> commit c4f1e37204aaea7efb2aa7dc234d5c8ebeba1089
> Author: Jason Merrill <jason@redhat.com>
> Date: Mon Mar 4 14:09:57 2019 -0500
>
> * gimplify.c (gimplify_cond_expr): Don't check TREE_ADDRESSABLE.
>
> The front end shouldn't produce a GENERIC COND_EXPR of TREE_ADDRESSABLE
> type.
I think this broke a lot in the D testsuite:
+FAIL: gdc.test/runnable/link15017.d (internal compiler error)
+UNRESOLVED: gdc.test/runnable/link15017.d compilation failed to produce executable
+FAIL: gdc.test/runnable/sdtor.d -O2 (internal compiler error)
+UNRESOLVED: gdc.test/runnable/sdtor.d -O2 compilation failed to produce executable
+FAIL: gdc.test/runnable/testaa.d (internal compiler error)
+UNRESOLVED: gdc.test/runnable/testaa.d compilation failed to produce executable
+FAIL: libphobos.druntime/rt/lifetime.d (test for excess errors)
+UNRESOLVED: libphobos.druntime/rt/lifetime.d compilation failed to produce executable
+FAIL: libphobos.druntime_shared/rt/lifetime.d (test for excess errors)
+UNRESOLVED: libphobos.druntime_shared/rt/lifetime.d compilation failed to produce executable
+FAIL: libphobos.phobos/std/uni.d (test for excess errors)
+UNRESOLVED: libphobos.phobos/std/uni.d compilation failed to produce executable
+FAIL: libphobos.phobos_shared/std/uni.d (test for excess errors)
+UNRESOLVED: libphobos.phobos_shared/std/uni.d compilation failed to produce executable
(the list is reduced, so that for each failing tests there aren't many
lines).
Iain, is that something that can be fixed on the D FE side?
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -3990,10 +3990,12 @@ gimplify_cond_expr (tree *expr_p, gimple_seq *pre_p, fallback_t fallback)
> tree result;
>
> /* If either an rvalue is ok or we do not require an lvalue, create the
> - temporary. But we cannot do that if the type is addressable. */
> + temporary. We cannot do that if the type is addressable, but
> + that should have been avoided before we got here. */
> if (((fallback & fb_rvalue) || !(fallback & fb_lvalue))
> - && !TREE_ADDRESSABLE (type))
> + && (flag_checking || !TREE_ADDRESSABLE (type)))
> {
> + gcc_assert (!TREE_ADDRESSABLE (type));
> if (gimplify_ctxp->allow_rhs_cond_expr
> /* If either branch has side effects or could trap, it can't be
> evaluated unconditionally. */
Jakub
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?:
2019-05-24 14:13 ` Jakub Jelinek
@ 2019-05-24 14:39 ` Jason Merrill
0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2019-05-24 14:39 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Iain Buclaw, gcc-patches List
On Fri, May 24, 2019 at 10:13 AM Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, May 22, 2019 at 05:59:56PM -0400, Jason Merrill wrote:
> > On Tue, May 7, 2019 at 4:43 PM Jason Merrill <jason@redhat.com> wrote:
> > >
> > > * typeck.c (build_static_cast_1): Use cp_build_addr_expr.
> > >
> > > For GCC 9 I fixed this bug with a patch to gimplify_cond_expr, but this
> > > function was also doing the wrong thing.
> > >
> > > Using build_address does not push the ADDR_EXPR down into the arms of a
> > > COND_EXPR, which we need for proper handling of conversion of an lvalue ?:
> > > to another reference type.
> >
> > And that allows the gimplifier to assert that we should never see a
> > COND_EXPR of addressable type.
> >
> > Tested x86_64-pc-linux-gnu, applying to trunk.
>
> > commit c4f1e37204aaea7efb2aa7dc234d5c8ebeba1089
> > Author: Jason Merrill <jason@redhat.com>
> > Date: Mon Mar 4 14:09:57 2019 -0500
> >
> > * gimplify.c (gimplify_cond_expr): Don't check TREE_ADDRESSABLE.
> >
> > The front end shouldn't produce a GENERIC COND_EXPR of TREE_ADDRESSABLE
> > type.
>
> I think this broke a lot in the D testsuite:
Thanks, reverted.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-24 14:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-07 20:43 [C++ PATCH] PR c++/86485 - -Wmaybe-unused with empty class ?: Jason Merrill
2019-05-22 19:46 ` Jason Merrill
2019-05-22 22:00 ` Jason Merrill
2019-05-24 14:13 ` Jakub Jelinek
2019-05-24 14:39 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).