On Wed Feb 16, 2022 at 4:06 AM +08, Jason Merrill wrote: > > Ah, I see. I found it a bit odd that gcc-commit-mklog auto-generated a > > subject with "c:", > > but I just went with it as I didn't know any better. Unfortunately, I > > can't change it now on the current thread. > > That came from this line in the testcase: > > > +/* PR c/25689 */ > > The PR should be c++/25689. Also, sometimes the bugzilla component > isn't the same as the area of the compiler you're changing; the latter > is what you want in the patch subject, so that the right people know to > review it.
Oh, I see. Thanks for the explanation. I've fixed the line.
> > Ah, I didn't notice that. Sorry about that! I'm kinda new to the whole
> > mailing list setup so there are some kinks I have to iron out.
>
> FWIW it's often easier to send the patch as an attachment.
Alright, I'll send patches as attachments instead. I originally sent
them as text as it is easier to comment on them.
> > - gcc_assert (TREE_CODE (call) == CALL_EXPR
> > - || TREE_CODE (call) == AGGR_INIT_EXPR
> > - || call == error_mark_node);
> > + if (TREE_CODE (call) != CALL_EXPR
> > + && TREE_CODE (call) != AGGR_INIT_EXPR
> > + && call != error_mark_node)
> > + return NULL_TREE;
> > return call;
>
> Note that since this can return error_mark_node...
>
> > }
> >
> > diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
> > index 0cb17a6a8ab..7a8f317af0d 100644
> > --- a/gcc/cp/semantics.cc
> > +++ b/gcc/cp/semantics.cc
> > @@ -815,6 +815,24 @@ finish_goto_stmt (tree destination)
> > return add_stmt (build_stmt (input_location, GOTO_EXPR, destination));
> > }
> >
> > +/* Returns true if CALL is a (possibly wrapped) CALL_EXPR or AGGR_INIT_EXPR
> > + to operator=() that is written as an operator expression. */
> > +static bool
> > +is_assignment_op_expr_p (tree call)
> > +{
> > + if (call == NULL_TREE)
> > + return false;
> > +
> > + call = extract_call_expr (call);
> > + if (call == NULL_TREE || !CALL_EXPR_OPERATOR_SYNTAX (call))
>
> ...you probably want to check for it here.
Good point. I've added the check.
> > +/* Test non-empty class */
> > +void f2(B b1, B b2)
> > +{
> > + if (b1 = 0); /* { dg-warning "suggest parentheses" } */
> > + if (b1 = 0.); /* { dg-warning "suggest parentheses" } */
> > + if (b1 = b2); /* { dg-warning "suggest parentheses" } */
> > + if (b1.operator= (0));
> > +
> > + /* Ideally, we wouldn't warn for non-empty classes using trivial
> > + operator= (below), but we currently do as it is a MODIFY_EXPR. */
> > + // if (b1.operator= (b2));
>
> You can avoid it by calling suppress_warning on that MODIFY_EXPR in
> build_over_call.
Unfortunately, that also affects the warning for if (b1 = b2) just 5
lines above. Both expressions seem to generate the same tree structure.
0001-c-Add-diagnostic-when-operator-is-used-as-truth-cond.patch
Description: application/mbox
