Warn on self assignments in C and C++ by introducing the
new -Wself-assign flag. This flag does not warn on self
initializations.
gcc/c-family/ChangeLog:
* c.opt: Add -Wself-assign for C and C++.
* c.opt.urls: Add URL for -Wself-assign.
gcc/c/ChangeLog:
* c-parser.cc (c_parser_expr_no_commas): Implement
self assignment checking for C.
gcc/cp/ChangeLog:
* parser.cc (cp_parser_assignment_expression): Implement
self assignment checking for C++.
gcc/ChangeLog:
* doc/invoke.texi: Document new -Wself-assign warning.
gcc/testsuite/ChangeLog:
* g++.dg/plugin/selfassign.cc: Rename warn_self_assign
to check_self_assign to avoid warning variable name issue.
* gcc.dg/plugin/selfassign.cc: Rename warn_self_assign
to check_self_assign to avoid warning variable name issue.
* g++.dg/Wself-assign-1.C: New C++ test for -Wself-assign.
* g++.dg/Wself-assign-2.C: New C++ test for -Wself-assign.
* g++.dg/Wself-assign-3.C: New C++ test for -Wself-assign.
* g++.dg/Wself-assign-4.C: New C++ test for -Wself-assign.
* g++.dg/Wself-assign-5.C: New C++ test for -Wself-assign.
* g++.dg/Wself-assign-6.C: New C++ test for -Wself-assign.
* gcc.dg/Wself-assign-1.c: New C test for -Wself-assign.
* gcc.dg/Wself-assign-2.c: New C test for -Wself-assign.
* gcc.dg/Wself-assign-3.c: New C test for -Wself-assign.
* gcc.dg/Wself-assign-4.c: New C test for -Wself-assign.
Signed-off-by: Neal Patalay <[email protected]>
---
Please note that this warning is currently not enabled by -Wall. Let
me know if it should be.
If this patch is approved, could someone please commit it and update
PR53129 for me?
Bootstrapped and regression tested on x86_64-pc-linux-gnu with no
regressions.
gcc/c-family/c.opt | 4 ++
gcc/c-family/c.opt.urls | 3 ++
gcc/c/c-parser.cc | 17 +++++++
gcc/cp/parser.cc | 24 ++++++++++
gcc/doc/invoke.texi | 21 ++++++++-
gcc/testsuite/g++.dg/Wself-assign-1.C | 55 +++++++++++++++++++++++
gcc/testsuite/g++.dg/Wself-assign-2.C | 31 +++++++++++++
gcc/testsuite/g++.dg/Wself-assign-3.C | 39 ++++++++++++++++
gcc/testsuite/g++.dg/Wself-assign-4.C | 48 ++++++++++++++++++++
gcc/testsuite/g++.dg/Wself-assign-5.C | 11 +++++
gcc/testsuite/g++.dg/Wself-assign-6.C | 8 ++++
gcc/testsuite/g++.dg/plugin/selfassign.cc | 4 +-
gcc/testsuite/gcc.dg/Wself-assign-1.c | 27 +++++++++++
gcc/testsuite/gcc.dg/Wself-assign-2.c | 25 +++++++++++
gcc/testsuite/gcc.dg/Wself-assign-3.c | 9 ++++
gcc/testsuite/gcc.dg/Wself-assign-4.c | 9 ++++
gcc/testsuite/gcc.dg/plugin/selfassign.cc | 4 +-
17 files changed, 334 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/Wself-assign-1.C
create mode 100644 gcc/testsuite/g++.dg/Wself-assign-2.C
create mode 100644 gcc/testsuite/g++.dg/Wself-assign-3.C
create mode 100644 gcc/testsuite/g++.dg/Wself-assign-4.C
create mode 100644 gcc/testsuite/g++.dg/Wself-assign-5.C
create mode 100644 gcc/testsuite/g++.dg/Wself-assign-6.C
create mode 100644 gcc/testsuite/gcc.dg/Wself-assign-1.c
create mode 100644 gcc/testsuite/gcc.dg/Wself-assign-2.c
create mode 100644 gcc/testsuite/gcc.dg/Wself-assign-3.c
create mode 100644 gcc/testsuite/gcc.dg/Wself-assign-4.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 0260b9dbf42..7d56e48d7fd 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1361,6 +1361,10 @@ Wselector
ObjC ObjC++ Var(warn_selector) Warning
Warn if a selector has multiple methods.
+Wself-assign
+C C++ Var(warn_self_assign) Warning
+Warn when an object is assigned to itself.
+
Wself-move
C++ ObjC++ Var(warn_self_move) Warning LangEnabledBy(C++ ObjC++, Wall)
Warn when a value is moved to itself with std::move.
diff --git a/gcc/c-family/c.opt.urls b/gcc/c-family/c.opt.urls
index 47e6c58f495..0564309ea04 100644
--- a/gcc/c-family/c.opt.urls
+++ b/gcc/c-family/c.opt.urls
@@ -948,6 +948,9 @@
UrlSuffix(gcc/Warning-Options.html#index-Wno-scalar-storage-order)
Wselector
UrlSuffix(gcc/Objective-C-and-Objective-C_002b_002b-Dialect-Options.html#index-Wno-selector)
+Wself-assign
+UrlSuffix(gcc/Warning-Options.html#index-Wself-assign)
+
Wself-move
UrlSuffix(gcc/Warning-Options.html#index-Wno-self-move)
diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 9dc5ba1e3a1..ed9f2ea23c1 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -10069,6 +10069,23 @@ c_parser_expr_no_commas (c_parser *parser, struct
c_expr *after,
ret.value = build_modify_expr (op_location, lhs.value, lhs.original_type,
code, exp_location, rhs.value,
rhs.original_type);
+
+ if (warn_self_assign
+ && !c_inhibit_evaluation_warnings
+ && code == NOP_EXPR
+ && lhs.value != error_mark_node
+ && rhs.value != error_mark_node
+ && ret.value != error_mark_node
+ && !TREE_SIDE_EFFECTS (lhs.value)
+ && !TREE_SIDE_EFFECTS (rhs.value)
+ && c_tree_equal (lhs.value, rhs.value))
+ {
+ location_t self_assign_loc = make_location (op_location, lhs.get_start
(),
+ rhs.get_finish ());
+ warning_at (self_assign_loc, OPT_Wself_assign,
+ "%qE is assigned to itself", lhs.value);
+ }
+
ret.m_decimal = 0;
set_c_expr_source_range (&ret, lhs.get_start (), rhs.get_finish ());
if (code == NOP_EXPR)
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 3f042bec1c6..d05bae7c633 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -12075,10 +12075,34 @@ cp_parser_assignment_expression (cp_parser* parser,
cp_id_kind * pidk,
loc = make_location (loc,
expr.get_start (),
rhs.get_finish ());
+
+ cp_expr lhs = expr;
+ tree stripped_lhs = tree_strip_any_location_wrapper
+ (maybe_undo_parenthesized_ref(expr));
+ tree stripped_rhs = tree_strip_any_location_wrapper
+ (maybe_undo_parenthesized_ref(rhs));
+
expr = build_x_modify_expr (loc, expr,
assignment_operator,
rhs, NULL_TREE,
complain_flags (decltype_p));
+
+ if (warn_self_assign
+ && !cp_unevaluated_operand
+ && assignment_operator == NOP_EXPR
+ && stripped_lhs != error_mark_node
+ && stripped_rhs != error_mark_node
+ && expr != error_mark_node
+ && !TREE_SIDE_EFFECTS (stripped_lhs)
+ && !TREE_SIDE_EFFECTS (stripped_rhs)
+ && cp_tree_equal (stripped_lhs, stripped_rhs)) {
+ location_t self_assign_loc = make_location (
+ loc,
+ lhs.get_start (),
+ cp_lexer_previous_token (parser->lexer)->location);
+ warning_at (self_assign_loc, OPT_Wself_assign,
+ "%qE is assigned to itself", stripped_lhs);
+ }
/* TODO: build_x_modify_expr doesn't honor the location,
so we must set it here. */
expr.set_location (loc);
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5be3851a929..5bc475cba5f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -436,7 +436,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-psabi
-Wredundant-decls -Wrestrict
-Wno-return-local-addr -Wreturn-type
--Wno-scalar-storage-order -Wsequence-point
+-Wno-scalar-storage-order -Wself-assign -Wsequence-point
-Wshadow -Wshadow=global -Wshadow=local -Wshadow=compatible-local
-Wno-shadow-ivar
-Wno-shift-count-negative -Wno-shift-count-overflow
@@ -8045,6 +8045,25 @@ of a declaration:
This warning is enabled by @option{-Wall}.
+@opindex Wself-assign
+@opindex Wno-self-assign
+@item -Wself-assign @r{(C and C++ only)}
+Warn when an object is assigned to itself. These assignments usually
+have no effect and can indicate a typo. This warning is not triggered
+for initializations.
+
+@smallexample
+void func()
+@{
+ int i = 1;
+ i = i; /* warning */
+ i = i + 0; /* no warning */
+ i += 0; /* no warning */
+@}
+@end smallexample
+
+This warning is enabled by @option{-Wself-assign} in C and C++.
+
@opindex Wself-move
@opindex Wno-self-move
@item -Wno-self-move @r{(C++ and Objective-C++ only)}
diff --git a/gcc/testsuite/g++.dg/Wself-assign-1.C
b/gcc/testsuite/g++.dg/Wself-assign-1.C
new file mode 100644
index 00000000000..05ad237c7a4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wself-assign-1.C
@@ -0,0 +1,55 @@
+/* Test self-assignment detection in various scenarios. */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+class Foo {
+ private:
+ int a_;
+
+ public:
+ Foo() : a_(a_) {} /* should not warn */
+
+ 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; /* should not warn */
+Foo foo = foo; /* should not warn */
+
+int func()
+{
+ Bar *bar1, bar2;
+ Foo local_foo;
+ int x = x; /* should not warn */
+ static int y = y; /* should not warn */
+ 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 */
+ return 0;
+}
diff --git a/gcc/testsuite/g++.dg/Wself-assign-2.C
b/gcc/testsuite/g++.dg/Wself-assign-2.C
new file mode 100644
index 00000000000..e397b8c2a83
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wself-assign-2.C
@@ -0,0 +1,31 @@
+/* Test the handling of expressions that depend on template parameters in
+ self-assignment detection. */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+template<typename T>
+struct Bar {
+ T x;
+ Bar operator++(int) {
+ Bar tmp = *this;
+ ++x;
+ tmp = tmp; /* { dg-warning "assigned to itself" } */
+ return tmp;
+ }
+};
+
+template<typename T>
+T DoSomething(T y) {
+ T a[5], *p;
+ Bar<T> 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;
+}
+
+int main() {
+ Bar<int> bar;
+ bar++;
+ DoSomething(5);
+}
diff --git a/gcc/testsuite/g++.dg/Wself-assign-3.C
b/gcc/testsuite/g++.dg/Wself-assign-3.C
new file mode 100644
index 00000000000..451faaf2877
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wself-assign-3.C
@@ -0,0 +1,39 @@
+/* Ensure identical assignments with temporaries or
+ potential side effects don't warn. */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+namespace testing {
+
+class Foo {
+ int f;
+ public:
+ Foo() {}
+};
+
+class Bar {
+ int b;
+ public:
+ Bar(int x) {}
+
+ void operator=(const Foo& foo) {}
+};
+
+}
+
+template <class T>
+void func(T t) {
+ ::testing::Bar(1) = ::testing::Foo();
+ ::testing::Foo() = ::testing::Foo(); /* should not warn */
+}
+
+int func2() {
+ return 0;
+}
+
+int main() {
+ int a[1];
+ a[func2()] = a[func2()]; /* should not warn */
+ a[0] = a[0]; /* { dg-warning "assigned to itself" } */
+ func(2);
+}
diff --git a/gcc/testsuite/g++.dg/Wself-assign-4.C
b/gcc/testsuite/g++.dg/Wself-assign-4.C
new file mode 100644
index 00000000000..be913d659ea
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wself-assign-4.C
@@ -0,0 +1,48 @@
+/* Test how self assignment detection handles constant folding
+ and parenthesis. */
+/* { 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 */
+
+void 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; /* should not warn */
+ y -= 0; /* should not warn */
+ foo = (foo); /* { dg-warning "assigned to itself" } */
+ foo.operator=(foo); /* should not warn */
+}
diff --git a/gcc/testsuite/g++.dg/Wself-assign-5.C
b/gcc/testsuite/g++.dg/Wself-assign-5.C
new file mode 100644
index 00000000000..8785b9975ad
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wself-assign-5.C
@@ -0,0 +1,11 @@
+/* Ensure unevaluated assignments don't warn. */
+/* { dg-do compile {target c++11} } */
+/* { dg-options "-Wself-assign" } */
+
+void f (int x)
+{
+ (void) sizeof (x = x); /* should not warn */
+ (void) noexcept (x = x); /* should not warn */
+ using T = decltype (x = x); /* should not warn */
+ x = x; /* { dg-warning "assigned to itself" } */
+}
diff --git a/gcc/testsuite/g++.dg/Wself-assign-6.C
b/gcc/testsuite/g++.dg/Wself-assign-6.C
new file mode 100644
index 00000000000..d3c108e8af1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Wself-assign-6.C
@@ -0,0 +1,8 @@
+/* Ensure invalid assignments don't warn */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+void f (int x)
+{
+ 1 = 1; /* { dg-error "lvalue required" } */
+}
diff --git a/gcc/testsuite/g++.dg/plugin/selfassign.cc
b/gcc/testsuite/g++.dg/plugin/selfassign.cc
index fd78f574307..1d0daa9b56d 100644
--- a/gcc/testsuite/g++.dg/plugin/selfassign.cc
+++ b/gcc/testsuite/g++.dg/plugin/selfassign.cc
@@ -210,7 +210,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;
@@ -289,7 +289,7 @@ pass_warn_self_assign::execute (function *fun)
FOR_EACH_BB_FN (bb, fun)
{
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 00000000000..e44f0da2760
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wself-assign-1.c
@@ -0,0 +1,27 @@
+/* Test self-assignment warning detection. */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+ int b_;
+ int c_;
+};
+
+int g;
+
+int main()
+{
+ struct Bar *bar;
+ int x = x; /* should not warn */
+ 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 00000000000..9cd57c57369
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wself-assign-2.c
@@ -0,0 +1,25 @@
+/* Test self-assignment detection with constant-folding and
+ parenthesis. */
+/* { 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 not warn */
+ g = g + 0; /* should not warn */
+ y = y / 1; /* should not warn */
+ bar->b_ = bar->b_ - 0; /* should not warn */
+ y = (y); /* { dg-warning "assigned to itself" } */
+}
diff --git a/gcc/testsuite/gcc.dg/Wself-assign-3.c
b/gcc/testsuite/gcc.dg/Wself-assign-3.c
new file mode 100644
index 00000000000..815ecb8839e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wself-assign-3.c
@@ -0,0 +1,9 @@
+/* Ensure unevaluated assignments don't warn */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+void f (int x)
+{
+ (void) sizeof (x = x); /* should not warn */
+ x = x; /* { dg-warning "assigned to itself" } */
+}
diff --git a/gcc/testsuite/gcc.dg/Wself-assign-4.c
b/gcc/testsuite/gcc.dg/Wself-assign-4.c
new file mode 100644
index 00000000000..ee30d6714d1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wself-assign-4.c
@@ -0,0 +1,9 @@
+/* Ensure invalid assignments don't warn. */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+void f (int x)
+{
+ 1 = 1; /* { dg-error "lvalue required" } */
+}
+
diff --git a/gcc/testsuite/gcc.dg/plugin/selfassign.cc
b/gcc/testsuite/gcc.dg/plugin/selfassign.cc
index 13b3ecaa0f2..4acef5f6079 100644
--- a/gcc/testsuite/gcc.dg/plugin/selfassign.cc
+++ b/gcc/testsuite/gcc.dg/plugin/selfassign.cc
@@ -210,7 +210,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;
@@ -288,7 +288,7 @@ pass_warn_self_assign::execute (function *fun)
FOR_EACH_BB_FN (bb, fun)
{
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;
--
2.55.0
