From patchwork Fri Apr 22 20:08:12 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Le-Chun Wu X-Patchwork-Id: 92579 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id C7F941007DC for ; Sat, 23 Apr 2011 06:08:42 +1000 (EST) Received: (qmail 9456 invoked by alias); 22 Apr 2011 20:08:40 -0000 Received: (qmail 9444 invoked by uid 22791); 22 Apr 2011 20:08:36 -0000 X-SWARE-Spam-Status: No, hits=-0.2 required=5.0 tests=AWL, BAYES_50, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 22 Apr 2011 20:08:17 +0000 Received: from wpaz1.hot.corp.google.com (wpaz1.hot.corp.google.com [172.24.198.65]) by smtp-out.google.com with ESMTP id p3MK8EUV010196; Fri, 22 Apr 2011 13:08:14 -0700 Received: from lcwu.mtv.corp.google.com (lcwu.mtv.corp.google.com [172.18.120.183]) by wpaz1.hot.corp.google.com with ESMTP id p3MK8CgN020241; Fri, 22 Apr 2011 13:08:13 -0700 Received: by lcwu.mtv.corp.google.com (Postfix, from userid 56351) id A7FC3184767; Fri, 22 Apr 2011 13:08:12 -0700 (PDT) To: reply@codereview.appspotmail.com, dnovillo@google.com, gcc-patches@gcc.gnu.org Subject: [google] Port self-assign warning to google/main branch (issue4442075) Message-Id: <20110422200812.A7FC3184767@lcwu.mtv.corp.google.com> Date: Fri, 22 Apr 2011 13:08:12 -0700 (PDT) From: lcwu@google.com (Le-Chun Wu) X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 2011-04-22 Le-Chun Wu gcc/c-family/ChangeLog: * c-common.c (check_for_self_assign): New function. * c-common.h: New function declaration. * c.opt: New option. gcc/ChangeLog: * c-parser.c (c_parser_declaration_or_fndef): Check for self-assign. (c_parser_expr_no_commas): Check for self-assign. * common.opt: New option. * doc/invoke.texi: Documentation for new options. * fold-const.c (operand_equal_p): Allow operands without typres to compare. (fold_unary_loc_1): Renamed from fold_unary_loc. (fold_unary_loc): New wrapper function. (fold_binary_loc_1): Renamed from fold_binary_loc. (fold_binary_loc): New wrapper function. (fold_ternary_loc_1): Renamed from fold_ternary_loc. (fold_ternary_loc): New wrapper function. * tree.h (struct tree_base): New flag for folded expr. (enum operand_equal_flag): New flags. gcc/cp/ChangeLog: * init.c (perform_member_init): Check for self-assign. * parser.c (expr_is_pod): New function. (cp_parser_assignment_expression): Check for self-assign. (cp_parser_init_declarator): Check for self-assign. gcc/testsuite/ChangeLog: * testsuite/g++.dg/plugin/selfassign.c (check_self_assign): Renamed from warn_self_assign. (execute_warn_self_assign): Call a function by its new name. * testsuite/g++.dg/warn/Wself-assign-1.C: New test case. * testsuite/g++.dg/warn/Wself-assign-2.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-3.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-4.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-5.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-non-pod-1.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-non-pod-2.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-non-pod-3.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-non-pod-4.C: Likewise. * testsuite/g++.dg/warn/Wself-assign-non-pod-5.C: Likewise. * testsuite/gcc.dg/plugin/selfassign.c (check_self_assign): Renamed from warn_self_assign. (execute_warn_self_assign): Call a function by its new name.: * testsuite/gcc.dg/wself-assign-1.c: New test case. * testsuite/gcc.dg/wself-assign-2.c: Likewise. --- This patch is available for review at http://codereview.appspot.com/4442075 diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index ca17bd2..13f13a8 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -10495,4 +10495,21 @@ keyword_is_decl_specifier (enum rid keyword) } } +/* Check for and warn about self-assignment or self-initialization. + LHS and RHS are the tree nodes for the left-hand side and right-hand side + of the assignment or initialization we are checking. + LOCATION is the source location for RHS. */ + +void +check_for_self_assign (location_t location, tree lhs, tree rhs) +{ + /* Only emit a warning if RHS is not a folded expression so that we don't + warn on something like x = x / 1. */ + if (!EXPR_FOLDED (rhs) + && operand_equal_p (lhs, rhs, + OEP_PURE_SAME | OEP_ALLOW_NULL | OEP_ALLOW_NO_TYPE)) + warning_at (location, OPT_Wself_assign, G_("%qE is assigned to itself"), + lhs); +} + #include "gt-c-family-c-common.h" diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 406def9..566aba7 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -957,6 +957,7 @@ extern VEC(tree,gc) *make_tree_vector (void); extern void release_tree_vector (VEC(tree,gc) *); extern VEC(tree,gc) *make_tree_vector_single (tree); extern VEC(tree,gc) *make_tree_vector_copy (const VEC(tree,gc) *); +extern void check_for_self_assign (location_t, tree, tree); /* In c-gimplify.c */ extern void c_genericize (tree); diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index bb928fa..af38a1f 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -319,6 +319,10 @@ Wconversion-null C++ ObjC++ Var(warn_conversion_null) Init(1) Warning Warn for converting NULL from/to a non-pointer type +Wself-assign-non-pod +C++ ObjC++ Var(warn_self_assign_non_pod) Init(0) Warning +Warn when a variable of a non-POD type is assigned to itself + Wsign-conversion C ObjC C++ ObjC++ Var(warn_sign_conversion) Init(-1) Warn for implicit type conversions between signed and unsigned integers diff --git a/gcc/c-parser.c b/gcc/c-parser.c index 25bd790..72a9f6c 100644 --- a/gcc/c-parser.c +++ b/gcc/c-parser.c @@ -1626,6 +1626,11 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok, maybe_warn_string_init (TREE_TYPE (d), init); finish_decl (d, init_loc, init.value, init.original_type, asm_name); + /* Check for and warn about self-initialization if + -Wself-assign is enabled. Don't warn if there is + already an error for the initializer. */ + if (warn_self_assign && DECL_INITIAL (d) != error_mark_node) + check_for_self_assign (here, d, init.value); } } else @@ -5241,7 +5246,13 @@ c_parser_expr_no_commas (c_parser *parser, struct c_expr *after) code, exp_location, rhs.value, rhs.original_type); if (code == NOP_EXPR) - ret.original_code = MODIFY_EXPR; + { + ret.original_code = MODIFY_EXPR; + /* Check for and warn about self-assignment if -Wself-assign is + enabled. */ + if (warn_self_assign) + check_for_self_assign (op_location, lhs.value, rhs.value); + } else { TREE_NO_WARNING (ret.value) = 1; diff --git a/gcc/common.opt b/gcc/common.opt index 273f17a..252bf68 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -561,6 +561,10 @@ Wpadded Common Var(warn_padded) Warning Warn when padding is required to align structure members +Wself-assign +Common Var(warn_self_assign) Init(0) Warning +Warn when a variable is assigned to itself + Wshadow Common Var(warn_shadow) Warning Warn when one local variable shadows another diff --git a/gcc/cp/init.c b/gcc/cp/init.c index e1961c8..bca8a03 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -598,8 +598,14 @@ perform_member_init (tree member, tree init) tf_warning_or_error); if (init) - finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init, - tf_warning_or_error)); + { + finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init, + tf_warning_or_error)); + /* Check for and warn about self-initialization if -Wself-assign is + enabled. */ + if (warn_self_assign) + check_for_self_assign (input_location, decl, init); + } } if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)) diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 08914d4..f08080c 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -7103,6 +7103,16 @@ cp_parser_question_colon_clause (cp_parser* parser, tree logical_or_expr) tf_warning_or_error); } +/* A helpfer function to check if the given expression (EXPR) is of POD type. + Note that if the expression's type is NULL (e.g. when its type depends on + template parameters), we return false. */ + +static bool +expr_is_pod (tree expr) +{ + return TREE_TYPE (expr) && pod_type_p (TREE_TYPE (expr)); +} + /* Parse an assignment-expression. assignment-expression: @@ -7158,6 +7168,16 @@ cp_parser_assignment_expression (cp_parser* parser, bool cast_p, if (cp_parser_non_integral_constant_expression (parser, NIC_ASSIGNMENT)) return error_mark_node; + + /* Check for and warn about self-assignment if -Wself-assign is + enabled and the assignment operator is "=". + Checking for non-POD self-assignment will be performed only + when -Wself-assign-non-pod is enabled. */ + if (warn_self_assign + && assignment_operator == NOP_EXPR + && (warn_self_assign_non_pod || expr_is_pod (expr))) + check_for_self_assign (input_location, expr, rhs); + /* Build the assignment expression. */ expr = build_x_modify_expr (expr, assignment_operator, @@ -14879,6 +14899,10 @@ cp_parser_init_declarator (cp_parser* parser, `explicit' constructor cannot be used. */ ((is_direct_init || !is_initialized) ? LOOKUP_NORMAL : LOOKUP_IMPLICIT)); + /* Check for and warn about self-initialization if -Wself-assign is + enabled. */ + if (warn_self_assign && initializer) + check_for_self_assign (input_location, decl, initializer); } else if ((cxx_dialect != cxx98) && friend_p && decl && TREE_CODE (decl) == FUNCTION_DECL) diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 45a99f8..b6e73f8 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -257,8 +257,8 @@ Objective-C and Objective-C++ Dialects}. -Woverlength-strings -Wpacked -Wpacked-bitfield-compat -Wpadded @gol -Wparentheses -Wpedantic-ms-format -Wno-pedantic-ms-format @gol -Wpointer-arith -Wno-pointer-to-int-cast @gol --Wredundant-decls @gol --Wreturn-type -Wsequence-point -Wshadow @gol +-Wredundant-decls -Wreturn-type @gol +-Wself-assign -Wself-assign-non-pod -Wsequence-point -Wshadow @gol -Wsign-compare -Wsign-conversion -Wstack-protector @gol -Wstrict-aliasing -Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol @@ -3379,6 +3379,61 @@ definitions, may be found on the GCC readings page, at This warning is enabled by @option{-Wall} for C and C++. +@item -Wself-assign +@opindex Wself-assign +@opindex Wno-self-assign +Warn about self-assignment and self-initialization. This warning is intended +for detecting accidental self-assignment due to typos, and therefore does +not warn on a statement that is semantically a self-assignment after +constant folding. Here is an example of what will trigger a self-assign +warning and what will not: + +@smallexample +@group +void func() +@{ + int i = 2; + int x = x; /* warn */ + float f = 5.0; + double a[3]; + + i = i + 0; /* not warn */ + f = f / 1; /* not warn */ + a[1] = a[1]; /* warn */ + i += 0; /* not warn */ +@} +@end group +@end smallexample + +In C++ it will not warn on self-assignment of non-POD variables unless +@option{-Wself-assign-non-pod} is also enabled. + +@item -Wself-assign-non-pod +@opindex Wself-assign-non-pod +@opindex Wno-self-assign-non-pod +Warn about self-assignment of non-POD variables. This is a C++-specific +warning and only effective when @option{-Wself-assign} is enabled. + +There are cases where self-assignment might be intentional. For example, +a C++ programmer might write code to test whether an overloaded +@code{operator=} works when the same object is assigned to itself. +One way to work around the self-assign warning in such cases when this flag +is enabled is using the functional form @code{object.operator=(object)} +instead of the assignment form @code{object = object}, as shown in the +following example. + +@smallexample +@group +void test_func() +@{ + MyType t; + + t.operator=(t); // not warn + t = t; // warn +@} +@end group +@end smallexample + @item -Wreturn-type @opindex Wreturn-type @opindex Wno-return-type @@ -3554,6 +3609,10 @@ This warning is enabled by @option{-Wall}. To suppress this warning use the @samp{unused} attribute (@pxref{Variable Attributes}). +Note that a classic way to avoid @option{-Wunused-variable} warning is +using @code{x = x}, but that does not work with @option{-Wself-assign}. +Use @code{(void) x} or @code{static_cast(x)} instead. + @item -Wunused-value @opindex Wunused-value @opindex Wno-unused-value diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 957049c..d82ddc7 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -2392,11 +2392,27 @@ combine_comparisons (location_t loc, If OEP_PURE_SAME is set, then pure functions with identical arguments are considered the same. It is used when the caller has other ways - to ensure that global memory is unchanged in between. */ + to ensure that global memory is unchanged in between. + + If OEP_ALLOW_NULL is set, this routine will not crash on NULL operands, + and two NULL operands are considered equal. This flag is usually set + in the context of frontend when ARG0 and/or ARG1 may be NULL mostly due + to recursion on partially built expressions (e.g. a CAST_EXPR on a NULL + tree.) In this case, we certainly don't want the compiler to crash and + it's OK to consider two NULL operands equal. On the other hand, when + called in the context of code generation and optimization, if NULL + operands are not expected, silently ignoring them could be dangerous + and might cause problems downstream that are hard to find/debug. In that + case, the flag should probably not be set. */ int operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) { + /* If either is NULL, they must be both NULL to be equal. We only do this + check when OEP_ALLOW_NULL is set. */ + if ((flags & OEP_ALLOW_NULL) && (!arg0 || !arg1)) + return arg0 == arg1; + /* If either is ERROR_MARK, they aren't equal. */ if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK || TREE_TYPE (arg0) == error_mark_node @@ -2406,7 +2422,13 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) /* Similar, if either does not have a type (like a released SSA name), they aren't equal. */ if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1)) - return 0; + { + /* If the caller chooses to allow the comparison of operands without + types, we will continue the comparison only when both of them don't + have a type. */ + if (!(flags & OEP_ALLOW_NO_TYPE) || TREE_TYPE (arg0) || TREE_TYPE (arg1)) + return 0; + } /* Check equality of integer constants before bailing out due to precision differences. */ @@ -2418,19 +2440,24 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) because they may change the signedness of the arguments. As pointers strictly don't have a signedness, require either two pointers or two non-pointers as well. */ - if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1)) - || POINTER_TYPE_P (TREE_TYPE (arg0)) != POINTER_TYPE_P (TREE_TYPE (arg1))) + if (TREE_TYPE (arg0) + && (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1)) + || POINTER_TYPE_P (TREE_TYPE (arg0)) + != POINTER_TYPE_P (TREE_TYPE (arg1)))) return 0; /* We cannot consider pointers to different address space equal. */ - if (POINTER_TYPE_P (TREE_TYPE (arg0)) && POINTER_TYPE_P (TREE_TYPE (arg1)) - && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0))) - != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1))))) + if (TREE_TYPE (arg0) + && (POINTER_TYPE_P (TREE_TYPE (arg0)) && POINTER_TYPE_P (TREE_TYPE (arg1)) + && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0))) + != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))) return 0; /* If both types don't have the same precision, then it is not safe to strip NOPs. */ - if (TYPE_PRECISION (TREE_TYPE (arg0)) != TYPE_PRECISION (TREE_TYPE (arg1))) + if (TREE_TYPE (arg0) + && (TYPE_PRECISION (TREE_TYPE (arg0)) + != TYPE_PRECISION (TREE_TYPE (arg1)))) return 0; STRIP_NOPS (arg0); @@ -2455,9 +2482,10 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) if (TREE_CODE (arg0) != TREE_CODE (arg1) /* This is needed for conversions and for COMPONENT_REF. Might as well play it safe and always test this. */ - || TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK - || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK - || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1))) + || (TREE_TYPE (arg0) + && (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK + || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK + || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1))))) return 0; /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal. @@ -2490,7 +2518,8 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) return 1; - if (!HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0)))) + if (TREE_TYPE (arg0) + && !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0)))) { /* If we do not distinguish between signed and unsigned zero, consider them equal. */ @@ -2558,8 +2587,9 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) { CASE_CONVERT: case FIX_TRUNC_EXPR: - if (TYPE_UNSIGNED (TREE_TYPE (arg0)) - != TYPE_UNSIGNED (TREE_TYPE (arg1))) + if (TREE_TYPE (arg0) + && (TYPE_UNSIGNED (TREE_TYPE (arg0)) + != TYPE_UNSIGNED (TREE_TYPE (arg1)))) return 0; break; default: @@ -2600,11 +2630,14 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) We can have incomplete types for array references of variable-sized arrays from the Fortran frontent though. */ - return ((TYPE_SIZE (TREE_TYPE (arg0)) == TYPE_SIZE (TREE_TYPE (arg1)) - || (TYPE_SIZE (TREE_TYPE (arg0)) - && TYPE_SIZE (TREE_TYPE (arg1)) - && operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)), - TYPE_SIZE (TREE_TYPE (arg1)), flags))) + return (TREE_TYPE (arg0) + && (TYPE_SIZE (TREE_TYPE (arg0)) + == TYPE_SIZE (TREE_TYPE (arg1)) + || (TYPE_SIZE (TREE_TYPE (arg0)) + && TYPE_SIZE (TREE_TYPE (arg1)) + && operand_equal_p (TYPE_SIZE (TREE_TYPE (arg0)), + TYPE_SIZE (TREE_TYPE (arg1)), + flags))) && (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg0, 1))) == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_OPERAND (arg1, 1)))) && OP_SAME (0) && OP_SAME (1)); @@ -7575,8 +7608,8 @@ build_fold_addr_expr_loc (location_t loc, tree t) OP0. Return the folded expression if folding is successful. Otherwise, return NULL_TREE. */ -tree -fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0) +static tree +fold_unary_loc_1 (location_t loc, enum tree_code code, tree type, tree op0) { tree tem; tree arg0; @@ -8223,6 +8256,41 @@ fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0) } /* switch (code) */ } +/* Given an expression tree EXP, set the EXPR_FOLDED flag, and if it is + a nop, recursively set the EXPR_FOLDED flag of its operand. */ + +static void +set_expr_folded_flag (tree exp) +{ + EXPR_FOLDED (exp) = 1; + + /* If EXP is a nop (i.e. NON_LVALUE_EXPRs and NOP_EXPRs), we need to + recursively set the EXPR_FOLDED flag of its operand because the + expression will be stripped later. */ + while ((CONVERT_EXPR_P (exp) + || TREE_CODE (exp) == NON_LVALUE_EXPR) + && TREE_OPERAND (exp, 0) != error_mark_node) + { + exp = TREE_OPERAND (exp, 0); + EXPR_FOLDED (exp) = 1; + } +} + +/* Fold a unary expression of code CODE and type TYPE with operand + OP0. Return the folded expression if folding is successful. + Otherwise, return NULL_TREE. + This is a wrapper around fold_unary_1 function (which does the + actual folding). Set the EXPR_FOLDED flag of the folded expression + if folding is successful. */ + +tree +fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0) +{ + tree tem = fold_unary_loc_1 (loc, code, type, op0); + if (tem) + set_expr_folded_flag (tem); + return tem; +} /* If the operation was a conversion do _not_ mark a resulting constant with TREE_OVERFLOW if the original constant was not. These conversions @@ -9328,8 +9396,8 @@ get_pointer_modulus_and_residue (tree expr, unsigned HOST_WIDE_INT *residue, Return the folded expression if folding is successful. Otherwise, return NULL_TREE. */ -tree -fold_binary_loc (location_t loc, +static tree +fold_binary_loc_1 (location_t loc, enum tree_code code, tree type, tree op0, tree op1) { enum tree_code_class kind = TREE_CODE_CLASS (code); @@ -13170,6 +13238,22 @@ fold_binary_loc (location_t loc, } /* switch (code) */ } +/* Fold a binary expression of code CODE and type TYPE with operands + OP0 and OP1. Return the folded expression if folding is + successful. Otherwise, return NULL_TREE. + This is a wrapper around fold_binary_1 function (which does the + actual folding). Set the EXPR_FOLDED flag of the folded expression + if folding is successful. */ +tree +fold_binary_loc (location_t loc, + enum tree_code code, tree type, tree op0, tree op1) +{ + tree tem = fold_binary_loc_1 (loc, code, type, op0, op1); + if (tem) + set_expr_folded_flag (tem); + return tem; +} + /* Callback for walk_tree, looking for LABEL_EXPR. Return *TP if it is a LABEL_EXPR; otherwise return NULL_TREE. Do not check the subtrees of GOTO_EXPR. */ @@ -13206,9 +13290,9 @@ contains_label_p (tree st) OP0, OP1, and OP2. Return the folded expression if folding is successful. Otherwise, return NULL_TREE. */ -tree -fold_ternary_loc (location_t loc, enum tree_code code, tree type, - tree op0, tree op1, tree op2) +static tree +fold_ternary_loc_1 (location_t loc, enum tree_code code, tree type, + tree op0, tree op1, tree op2) { tree tem; tree arg0 = NULL_TREE, arg1 = NULL_TREE, arg2 = NULL_TREE; @@ -13559,6 +13643,23 @@ fold_ternary_loc (location_t loc, enum tree_code code, tree type, } /* switch (code) */ } +/* Fold a ternary expression of code CODE and type TYPE with operands + OP0, OP1, and OP2. Return the folded expression if folding is + successful. Otherwise, return NULL_TREE. + This is a wrapper around fold_ternary_1 function (which does the + actual folding). Set the EXPR_FOLDED flag of the folded expression + if folding is successful. */ + +tree +fold_ternary_loc (location_t loc, enum tree_code code, tree type, + tree op0, tree op1, tree op2) +{ + tree tem = fold_ternary_loc_1 (loc, code, type, op0, op1, op2); + if (tem) + set_expr_folded_flag (tem); + return tem; +} + /* Perform constant folding and related simplification of EXPR. The related simplifications include x*1 => x, x*0 => 0, etc., and application of the associative law. diff --git a/gcc/testsuite/g++.dg/plugin/selfassign.c b/gcc/testsuite/g++.dg/plugin/selfassign.c index 84d2801..d40dfe3 100644 --- a/gcc/testsuite/g++.dg/plugin/selfassign.c +++ b/gcc/testsuite/g++.dg/plugin/selfassign.c @@ -194,7 +194,7 @@ compare_and_warn (gimple stmt, tree lhs, tree rhs) /* Check and warn if STMT is a self-assign statement. */ static void -warn_self_assign (gimple stmt) +check_self_assign (gimple stmt) { tree rhs, lhs; @@ -247,7 +247,7 @@ execute_warn_self_assign (void) FOR_EACH_BB (bb) { for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) - warn_self_assign (gsi_stmt (gsi)); + check_self_assign (gsi_stmt (gsi)); } return 0; diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-1.C b/gcc/testsuite/g++.dg/warn/Wself-assign-1.C new file mode 100644 index 0000000..f152805 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-1.C @@ -0,0 +1,54 @@ +// Test the self-assignemnt detection and warning. +// { dg-do compile } +// { dg-options "-Wself-assign" } + +class Foo { + private: + int a_; + + public: + Foo() : a_(a_) {} // { dg-warning "assigned to itself" } + + void setA(int a) { + a_ = a_; // { dg-warning "assigned to itself" } + } + + void operator=(Foo& rhs) { + this->a_ = rhs.a_; + } +}; + +struct Bar { + int b_; + int c_; +}; + +int g = g; // { dg-warning "assigned to itself" } +Foo foo = foo; // { dg-warning "assigned to itself" } + +int func() +{ + Bar *bar1, bar2; + Foo local_foo; + int x = x; // { dg-warning "assigned to itself" } + static int y = y; // { dg-warning "assigned to itself" } + float *f; + Bar bar_array[5]; + char n; + int overflow; + + *f = *f; // { dg-warning "assigned to itself" } + bar1->b_ = bar1->b_; // { dg-warning "assigned to itself" } + bar2.c_ = bar2.c_; // { dg-warning "assigned to itself" } + local_foo = local_foo; + foo = foo; + foo.setA(5); + bar_array[3].c_ = bar_array[3].c_; // { dg-warning "assigned to itself" } + bar_array[x+g].b_ = bar_array[x+g].b_; // { dg-warning "assigned to itself" } + y = x; + x = y; + x += 0; // should not warn + y -= 0; // should not warn + x /= x; // should not warn + y *= y; // should not warn +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-2.C b/gcc/testsuite/g++.dg/warn/Wself-assign-2.C new file mode 100644 index 0000000..35c3d84 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-2.C @@ -0,0 +1,31 @@ +// Test the handling of expressions that depend on template parameters in +// self-assignemnt detection. +// { dg-do compile } +// { dg-options "-Wself-assign" } + +template +struct Bar { + T x; + Bar operator++(int) { + Bar tmp = *this; + ++x; + tmp = tmp; // { dg-warning "assigned to itself" } + return tmp; + } +}; + +template +T DoSomething(T y) { + T a[5], *p; + Bar b; + b.x = b.x; + *p = *p; + a[2] = a[2]; + return *p; +} + +main() { + Bar bar; + bar++; + DoSomething(5); +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-3.C b/gcc/testsuite/g++.dg/warn/Wself-assign-3.C new file mode 100644 index 0000000..bc5732d --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-3.C @@ -0,0 +1,35 @@ +// Test how operands_equal_p handles a NULL operand. +// { dg-do compile } +// { dg-options "-Wself-assign" } + +#include + +namespace testing { + +class Foo { + int f; + public: + Foo() { printf("Construct Foo\n"); } +}; + +class Bar { + int b; + public: + Bar(int x) { printf("Construct Bar\n"); } + + void operator=(const Foo& foo) { + printf("Assign Foo to Bar\n"); + } +}; + +} + +template +void func(T t) { + ::testing::Bar(1) = ::testing::Foo(); // used to trigger a segfault + ::testing::Foo() = ::testing::Foo(); +} + +main() { + func(2); +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-4.C b/gcc/testsuite/g++.dg/warn/Wself-assign-4.C new file mode 100644 index 0000000..abe96b5 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-4.C @@ -0,0 +1,48 @@ +// Test how self-assignment detection handles constant-folding happening +// when parsing the RHS or the initializer. +// { dg-do compile } +// { dg-options "-Wself-assign" } + +class Foo { + private: + int a_; + + public: + Foo() : a_(a_+0) {} // should not warn + + void setA(int a) { + a_ = a_ + 0; // should not warn + } + + void operator=(Foo& rhs) { + this->a_ = rhs.a_; + } +}; + +struct Bar { + int b_; + float c_; +}; + +int g = g * 1; // should not warn + +int func() +{ + Bar *bar1, bar2; + Foo foo; + int x = x - 0; // should not warn + static int y = y / 1; // should not warn + float *f; + Bar bar_array[5]; + + *f = *f / 1; // should not warn + bar1->b_ = bar1->b_ * 1; // should not warn + bar2.c_ = bar2.c_ - 0; // should not warn + foo.setA(5); + bar_array[3].c_ = bar_array[3].c_ * 1; // should not warn + bar_array[x+g].b_ = bar_array[x+g].b_ / 1; // should not warn + x += 0; + y -= 0; + foo = foo; + foo.operator=(foo); // should not warn +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-5.C b/gcc/testsuite/g++.dg/warn/Wself-assign-5.C new file mode 100644 index 0000000..20df214 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-5.C @@ -0,0 +1,38 @@ +// Test -Wself-assign does not warn on self-assignment of non-POD variables. +// { dg-do compile } +// { dg-options "-Wself-assign" } + +template +class Foo { + private: + T a_; + public: + Foo() : a_(a_) {} // { dg-warning "assigned to itself" } + void Set() { a_ = a_; } +}; + +struct Bar { + int b_; + int c_; + void operator=(Bar& rhs) { + this->b_ = rhs.b_; + this->c_ = rhs.c_; + } +}; + +template +void func() { + T a; + a = a; +} + +main() +{ + Foo foo; + Bar *bar1, bar2; + func(); + foo = foo; + bar2 = bar2; + bar1 = bar1; // { dg-warning "assigned to itself" } + bar2.b_ = bar2.b_; // { dg-warning "assigned to itself" } +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-1.C b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-1.C new file mode 100644 index 0000000..6f9dfb7 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-1.C @@ -0,0 +1,54 @@ +// Test the self-assignemnt detection and warning. +// { dg-do compile } +// { dg-options "-Wself-assign -Wself-assign-non-pod" } + +class Foo { + private: + int a_; + + public: + Foo() : a_(a_) {} // { dg-warning "assigned to itself" } + + void setA(int a) { + a_ = a_; // { dg-warning "assigned to itself" } + } + + void operator=(Foo& rhs) { + this->a_ = rhs.a_; + } +}; + +struct Bar { + int b_; + int c_; +}; + +int g = g; // { dg-warning "assigned to itself" } +Foo foo = foo; // { dg-warning "assigned to itself" } + +int func() +{ + Bar *bar1, bar2; + Foo local_foo; + int x = x; // { dg-warning "assigned to itself" } + static int y = y; // { dg-warning "assigned to itself" } + float *f; + Bar bar_array[5]; + char n; + int overflow; + + *f = *f; // { dg-warning "assigned to itself" } + bar1->b_ = bar1->b_; // { dg-warning "assigned to itself" } + bar2.c_ = bar2.c_; // { dg-warning "assigned to itself" } + local_foo = local_foo; // { dg-warning "assigned to itself" } + foo = foo; // { dg-warning "assigned to itself" } + foo.setA(5); + bar_array[3].c_ = bar_array[3].c_; // { dg-warning "assigned to itself" } + bar_array[x+g].b_ = bar_array[x+g].b_; // { dg-warning "assigned to itself" } + y = x; + x = y; + x += 0; // should not warn + y -= 0; // should not warn + x /= x; // should not warn + y *= y; // should not warn +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-2.C b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-2.C new file mode 100644 index 0000000..b31b575 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-2.C @@ -0,0 +1,31 @@ +// Test the handling of expressions that depend on template parameters in +// self-assignemnt detection. +// { dg-do compile } +// { dg-options "-Wself-assign -Wself-assign-non-pod" } + +template +struct Bar { + T x; + Bar operator++(int) { + Bar tmp = *this; + ++x; + tmp = tmp; // { dg-warning "assigned to itself" } + return tmp; + } +}; + +template +T DoSomething(T y) { + T a[5], *p; + Bar b; + b.x = b.x; // { dg-warning "assigned to itself" } + *p = *p; // { dg-warning "assigned to itself" } + a[2] = a[2]; // { dg-warning "assigned to itself" } + return *p; +} + +main() { + Bar bar; + bar++; + DoSomething(5); +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-3.C b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-3.C new file mode 100644 index 0000000..4c37f57 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-3.C @@ -0,0 +1,35 @@ +// Test how operands_equal_p handles a NULL operand. +// { dg-do compile } +// { dg-options "-Wself-assign -Wself-assign-non-pod" } + +#include + +namespace testing { + +class Foo { + int f; + public: + Foo() { printf("Construct Foo\n"); } +}; + +class Bar { + int b; + public: + Bar(int x) { printf("Construct Bar\n"); } + + void operator=(const Foo& foo) { + printf("Assign Foo to Bar\n"); + } +}; + +} + +template +void func(T t) { + ::testing::Bar(1) = ::testing::Foo(); // used to trigger a segfault + ::testing::Foo() = ::testing::Foo(); // { dg-warning "assigned to itself" } +} + +main() { + func(2); +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-4.C b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-4.C new file mode 100644 index 0000000..86db4e3 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-4.C @@ -0,0 +1,48 @@ +// Test how self-assignment detection handles constant-folding happening +// when parsing the RHS or the initializer. +// { dg-do compile } +// { dg-options "-Wself-assign -Wself-assign-non-pod" } + +class Foo { + private: + int a_; + + public: + Foo() : a_(a_+0) {} // should not warn + + void setA(int a) { + a_ = a_ + 0; // should not warn + } + + void operator=(Foo& rhs) { + this->a_ = rhs.a_; + } +}; + +struct Bar { + int b_; + float c_; +}; + +int g = g * 1; // should not warn + +int func() +{ + Bar *bar1, bar2; + Foo foo; + int x = x - 0; // should not warn + static int y = y / 1; // should not warn + float *f; + Bar bar_array[5]; + + *f = *f / 1; // should not warn + bar1->b_ = bar1->b_ * 1; // should not warn + bar2.c_ = bar2.c_ - 0; // should not warn + foo.setA(5); + bar_array[3].c_ = bar_array[3].c_ * 1; // should not warn + bar_array[x+g].b_ = bar_array[x+g].b_ / 1; // should not warn + x += 0; + y -= 0; + foo = foo; // { dg-warning "assigned to itself" } + foo.operator=(foo); // should not warn +} diff --git a/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-5.C b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-5.C new file mode 100644 index 0000000..898ddec --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-assign-non-pod-5.C @@ -0,0 +1,38 @@ +// Test -Wself-assign and -Wself-assign-non-pod. +// { dg-do compile } +// { dg-options "-Wself-assign -Wself-assign-non-pod" } + +template +class Foo { + private: + T a_; + public: + Foo() : a_(a_) {} // { dg-warning "assigned to itself" } + void Set() { a_ = a_; } // { dg-warning "assigned to itself" } +}; + +struct Bar { + int b_; + int c_; + void operator=(Bar& rhs) { + this->b_ = rhs.b_; + this->c_ = rhs.c_; + } +}; + +template +void func() { + T a; + a = a; // { dg-warning "assigned to itself" } +} + +main() +{ + Foo foo; + Bar *bar1, bar2; + func(); + foo = foo; // { dg-warning "assigned to itself" } + bar2 = bar2; // { dg-warning "assigned to itself" } + bar1 = bar1; // { dg-warning "assigned to itself" } + bar2.b_ = bar2.b_; // { dg-warning "assigned to itself" } +} diff --git a/gcc/testsuite/gcc.dg/plugin/selfassign.c b/gcc/testsuite/gcc.dg/plugin/selfassign.c index 84d2801..d40dfe3 100644 --- a/gcc/testsuite/gcc.dg/plugin/selfassign.c +++ b/gcc/testsuite/gcc.dg/plugin/selfassign.c @@ -194,7 +194,7 @@ compare_and_warn (gimple stmt, tree lhs, tree rhs) /* Check and warn if STMT is a self-assign statement. */ static void -warn_self_assign (gimple stmt) +check_self_assign (gimple stmt) { tree rhs, lhs; @@ -247,7 +247,7 @@ execute_warn_self_assign (void) FOR_EACH_BB (bb) { for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) - warn_self_assign (gsi_stmt (gsi)); + check_self_assign (gsi_stmt (gsi)); } return 0; diff --git a/gcc/testsuite/gcc.dg/wself-assign-1.c b/gcc/testsuite/gcc.dg/wself-assign-1.c new file mode 100644 index 0000000..19324a6 --- /dev/null +++ b/gcc/testsuite/gcc.dg/wself-assign-1.c @@ -0,0 +1,27 @@ +/* Test the self-assignemnt detection and warning. */ +/* { dg-do compile } */ +/* { dg-options "-Wself-assign" } */ + +struct Bar { + int b_; + int c_; +}; + +int g; + +int main() +{ + struct Bar *bar; + int x = x; /* { dg-warning "assigned to itself" } */ + static int y; + struct Bar b_array[5]; + + b_array[x+g].b_ = b_array[x+g].b_; /* { dg-warning "assigned to itself" } */ + g = g; /* { dg-warning "assigned to itself" } */ + y = y; /* { dg-warning "assigned to itself" } */ + bar->b_ = bar->b_; /* { dg-warning "assigned to itself" } */ + x += 0; /* should not warn */ + y -= 0; /* should not warn */ + x /= x; /* should not warn */ + y *= y; /* should not warn */ +} diff --git a/gcc/testsuite/gcc.dg/wself-assign-2.c b/gcc/testsuite/gcc.dg/wself-assign-2.c new file mode 100644 index 0000000..d0f69cb --- /dev/null +++ b/gcc/testsuite/gcc.dg/wself-assign-2.c @@ -0,0 +1,24 @@ +/* Test how self-assignment detection handles constant-folding happening */ +/* when parsing the RHS or the initializer. */ +/* { dg-do compile } */ +/* { dg-options "-Wself-assign" } */ + +struct Bar { + int b_; + float c_; +}; + +int g; + +int main() +{ + struct Bar *bar; + int x = x - 0; /* should not warn */ + static int y; + struct Bar b_array[5]; + + b_array[x+g].b_ = b_array[x+g].b_ * 1; /* should no warn */ + g = g + 0; /* should not warn */ + y = y / 1; /* should not warn */ + bar->b_ = bar->b_ - 0; /* should not warn */ +} diff --git a/gcc/tree.h b/gcc/tree.h index 228cfbc..a58f1a7 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -398,8 +398,9 @@ struct GTY(()) tree_base { unsigned packed_flag : 1; unsigned user_align : 1; unsigned nameless_flag : 1; + unsigned expr_folded_flag : 1; - unsigned spare : 12; + unsigned spare : 11; /* This field is only used with type nodes; the only reason it is present in tree_base instead of tree_type is to save space. The size of the @@ -638,6 +639,13 @@ struct GTY(()) tree_common { SSA_NAME_IS_DEFAULT_DEF in SSA_NAME + + expr_folded_flag: + + EXPR_FOLDED in + all expressions + all decls + all constants */ #undef DEFTREESTRUCT @@ -1371,6 +1379,10 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int, /* In fixed-point types, means a saturating type. */ #define TYPE_SATURATING(NODE) ((NODE)->base.saturating_flag) +/* Nonzero in an expression, a decl, or a constant node if the node is + the result of a successful constant-folding. */ +#define EXPR_FOLDED(NODE) ((NODE)->base.expr_folded_flag) + /* These flags are available for each language front end to use internally. */ #define TREE_LANG_FLAG_0(NODE) ((NODE)->base.lang_flag_0) #define TREE_LANG_FLAG_1(NODE) ((NODE)->base.lang_flag_1) @@ -5056,7 +5068,10 @@ extern tree fold_fma (location_t, tree, tree, tree, tree); enum operand_equal_flag { OEP_ONLY_CONST = 1, - OEP_PURE_SAME = 2 + OEP_PURE_SAME = 2, + OEP_ALLOW_NULL = 4, /* Allow NULL operands to be passed in and compared. */ + OEP_ALLOW_NO_TYPE = 8 /* Allow operands both of which don't have a type + to be compared. */ }; extern int operand_equal_p (const_tree, const_tree, unsigned int);