public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ Patch] PR 14710 (add -Wuseless-cast)
@ 2012-03-16 21:45 Paolo Carlini
  2012-03-17 16:04 ` Jason Merrill
  0 siblings, 1 reply; 7+ messages in thread
From: Paolo Carlini @ 2012-03-16 21:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

Hi,

this is what I did to add the warning requested in the PR. Among the 
slightly less trivial bits:

1- In order not to risk warning *before* an hard error is produced by 
build_*_cast_1 code, I'm warning only if the cast is actually 
successful, thus at the end of each build_*_cast function.

2- In order not to warn when the useless cast shows up upon template 
instantiation I'm using here too the c_inhibit_evaluation_warnings trick 
(as we do already for warn_sign_compare, I believe).

3- References can be easily missed because wrapped in INDIRECT_REF: as 
explained at the beginning of tree.c and already used in many places, a 
REFERENCE_REF_P check (and in case a TREE_OPERAND (expr, 0)) takes care 
of that. I'm not 100% sure the solution is fully general, though.

4- I'm naming the function implementing the warning itself 
maybe_warn_about_useless_cast, which seems consistent with the name of 
at least another warning in typeck.c but other options are possible, of 
course...

Booted and tested x86_64-linux.

Thanks,
Paolo.

//////////////////////


[-- Attachment #2: CL_14710 --]
[-- Type: text/plain, Size: 791 bytes --]

2012-03-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/14710
	* doc/invoke.texi: Document -Wuseless-cast.

/c-family
2012-03-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/14710
	* c.opt ([Wuseless-cast]): Add.

/cp
2012-03-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/14710
	* cp-tree.h (maybe_warn_about_useless_cast): Declare.
	* typeck.c (maybe_warn_about_useless_cast): Define.
	(build_reinterpret_cast, build_const_cast,
	build_static_cast, cp_build_c_cast): Use it.
	* rtti.c (build_dynamic_cast): Likewise.
	* pt.c (tsubst_copy_and_build, case CAST_EXPR): Increment/decrement
	c_inhibit_evaluation_warnings before/after the build_* calls.

/testsuite
2012-03-16  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/14710
	* g++.dg/warn/Wuseless-cast.C: New.

[-- Attachment #3: patch_14710 --]
[-- Type: text/plain, Size: 10395 bytes --]

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 185462)
+++ doc/invoke.texi	(working copy)
@@ -274,8 +274,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wunused-label  -Wunused-local-typedefs -Wunused-parameter @gol
 -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol
 -Wunused-but-set-parameter -Wunused-but-set-variable @gol
--Wvariadic-macros -Wvector-operation-performance -Wvla 
--Wvolatile-register-var  -Wwrite-strings -Wzero-as-null-pointer-constant}
+-Wuseless-cast -Wvariadic-macros -Wvector-operation-performance @gol
+-Wvla -Wvolatile-register-var  -Wwrite-strings -Wzero-as-null-pointer-constant}
 
 @item C and Objective-C-only Warning Options
 @gccoptlist{-Wbad-function-cast  -Wmissing-declarations @gol
@@ -4199,6 +4199,11 @@ types. @option{-Wconversion-null} is enabled by de
 Warn when a literal '0' is used as null pointer constant.  This can
 be useful to facilitate the conversion to @code{nullptr} in C++11.
 
+@item -Wuseless-cast @r{(C++ and Objective-C++ only)}
+@opindex Wuseless-cast
+@opindex Wno-useless-cast
+Warn when an expression is casted to its own type.
+
 @item -Wempty-body
 @opindex Wempty-body
 @opindex Wno-empty-body
Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 185462)
+++ c-family/c.opt	(working copy)
@@ -697,6 +697,10 @@ Wzero-as-null-pointer-constant
 C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
 Warn when a literal '0' is used as null pointer
 
+Wuseless-cast
+C++ ObjC++ Var(warn_useless_cast) Warning
+Warn about useless casts
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++)
Index: testsuite/g++.dg/warn/Wuseless-cast.C
===================================================================
--- testsuite/g++.dg/warn/Wuseless-cast.C	(revision 0)
+++ testsuite/g++.dg/warn/Wuseless-cast.C	(revision 0)
@@ -0,0 +1,62 @@
+// { dg-options "-Wuseless-cast" }
+
+template<typename T>
+  void tmpl_f1(T t)
+  {
+    (int)(t);
+    static_cast<int>(t);
+    reinterpret_cast<int>(t);
+  }
+
+struct A { };
+
+template<typename T>
+  void tmpl_f2(T& t)
+  {
+    (A*)(&t);
+    const_cast<A*>(&t);
+    static_cast<A*>(&t);
+    reinterpret_cast<A*>(&t);
+    dynamic_cast<A*>(&t);
+  }
+
+template<typename T>
+  void tmpl_f3(T& t)
+  {
+    (A&)(t);
+    const_cast<A&>(t);
+    static_cast<A&>(t);
+    reinterpret_cast<A&>(t);
+    dynamic_cast<A&>(t);
+  }
+
+void f()
+{
+  int n; 
+
+  (int)(n);                   // { dg-warning "useless cast" }
+  static_cast<int>(n);        // { dg-warning "useless cast" }
+  reinterpret_cast<int>(n);   // { dg-warning "useless cast" }
+
+  tmpl_f1(n);
+
+  A a;
+
+  (A*)(&a);                   // { dg-warning "useless cast" }
+  const_cast<A*>(&a);         // { dg-warning "useless cast" }
+  static_cast<A*>(&a);        // { dg-warning "useless cast" }
+  reinterpret_cast<A*>(&a);   // { dg-warning "useless cast" }
+  dynamic_cast<A*>(&a);       // { dg-warning "useless cast" }
+
+  tmpl_f2(a);
+
+  A& b = a;
+
+  (A&)(b);                    // { dg-warning "useless cast" } 
+  const_cast<A&>(b);          // { dg-warning "useless cast" }
+  static_cast<A&>(b);         // { dg-warning "useless cast" }
+  reinterpret_cast<A&>(b);    // { dg-warning "useless cast" }
+  dynamic_cast<A&>(b);        // { dg-warning "useless cast" }
+
+  tmpl_f3(b);
+}
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 185473)
+++ cp/typeck.c	(working copy)
@@ -5749,6 +5749,24 @@ check_for_casting_away_constness (tree src_type, t
     }
 }
 
+/*
+  Warns if the cast from expression EXPR to type TYPE is useless.
+ */
+void
+maybe_warn_about_useless_cast (tree type, tree expr, tsubst_flags_t complain)
+{
+  if (complain & tf_warning
+      && c_inhibit_evaluation_warnings == 0)
+    {
+      if (REFERENCE_REF_P (expr))
+	expr = TREE_OPERAND (expr, 0);
+
+      if (same_type_p (TREE_TYPE (expr), type))
+	warning (OPT_Wuseless_cast, "useless cast from type %qT to itself",
+		 type);
+    }
+}
+
 /* Convert EXPR (an expression with pointer-to-member type) to TYPE
    (another pointer-to-member type in the same hierarchy) and return
    the converted expression.  If ALLOW_INVERSE_P is permitted, a
@@ -6078,7 +6096,11 @@ build_static_cast (tree type, tree expr, tsubst_fl
   result = build_static_cast_1 (type, expr, /*c_cast_p=*/false, &valid_p,
                                 complain);
   if (valid_p)
-    return result;
+    {
+      if (result != error_mark_node)
+	maybe_warn_about_useless_cast (type, expr, complain);
+      return result;
+    }
 
   if (complain & tf_error)
     error ("invalid static_cast from type %qT to type %qT",
@@ -6305,6 +6327,8 @@ build_reinterpret_cast_1 (tree type, tree expr, bo
 tree
 build_reinterpret_cast (tree type, tree expr, tsubst_flags_t complain)
 {
+  tree r;
+
   if (type == error_mark_node || expr == error_mark_node)
     return error_mark_node;
 
@@ -6319,8 +6343,11 @@ build_reinterpret_cast (tree type, tree expr, tsub
       return convert_from_reference (t);
     }
 
-  return build_reinterpret_cast_1 (type, expr, /*c_cast_p=*/false,
-				   /*valid_p=*/NULL, complain);
+  r = build_reinterpret_cast_1 (type, expr, /*c_cast_p=*/false,
+				/*valid_p=*/NULL, complain);
+  if (r != error_mark_node)
+    maybe_warn_about_useless_cast (type, expr, complain);
+  return r;
 }
 
 /* Perform a const_cast from EXPR to TYPE.  If the cast is valid,
@@ -6464,6 +6491,8 @@ build_const_cast_1 (tree dst_type, tree expr, tsub
 tree
 build_const_cast (tree type, tree expr, tsubst_flags_t complain)
 {
+  tree r;
+
   if (type == error_mark_node || error_operand_p (expr))
     return error_mark_node;
 
@@ -6478,8 +6507,10 @@ build_const_cast (tree type, tree expr, tsubst_fla
       return convert_from_reference (t);
     }
 
-  return build_const_cast_1 (type, expr, complain,
-			     /*valid_p=*/NULL);
+  r = build_const_cast_1 (type, expr, complain, /*valid_p=*/NULL);
+  if (r != error_mark_node)
+    maybe_warn_about_useless_cast (type, expr, complain);
+  return r;
 }
 
 /* Like cp_build_c_cast, but for the c-common bits.  */
@@ -6567,7 +6598,11 @@ cp_build_c_cast (tree type, tree expr, tsubst_flag
   result = build_const_cast_1 (type, value, complain & tf_warning,
 			       &valid_p);
   if (valid_p)
-    return result;
+    {
+      if (result != error_mark_node)
+	maybe_warn_about_useless_cast (type, value, complain);
+      return result;
+    }
 
   /* Or a static cast.  */
   result = build_static_cast_1 (type, value, /*c_cast_p=*/true,
@@ -6580,11 +6615,13 @@ cp_build_c_cast (tree type, tree expr, tsubst_flag
      const_cast.  */
   if (valid_p
       /* A valid cast may result in errors if, for example, a
-	 conversion to am ambiguous base class is required.  */
+	 conversion to an ambiguous base class is required.  */
       && !error_operand_p (result))
     {
       tree result_type;
 
+      maybe_warn_about_useless_cast (type, value, complain);
+
       /* Non-class rvalues always have cv-unqualified type.  */
       if (!CLASS_TYPE_P (type))
 	type = TYPE_MAIN_VARIANT (type);
Index: cp/rtti.c
===================================================================
--- cp/rtti.c	(revision 185473)
+++ cp/rtti.c	(working copy)
@@ -1,6 +1,6 @@
 /* RunTime Type Identification
    Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-   2005, 2006, 2007, 2008, 2009, 2010, 2011
+   2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
    Free Software Foundation, Inc.
    Mostly written by Jason Merrill (jason@cygnus.com).
 
@@ -774,6 +774,8 @@ build_dynamic_cast_1 (tree type, tree expr, tsubst
 tree
 build_dynamic_cast (tree type, tree expr, tsubst_flags_t complain)
 {
+  tree r;
+
   if (type == error_mark_node || expr == error_mark_node)
     return error_mark_node;
 
@@ -784,9 +786,12 @@ build_dynamic_cast (tree type, tree expr, tsubst_f
       return convert_from_reference (expr);
     }
 
-  return convert_from_reference (build_dynamic_cast_1 (type, expr, complain));
+  r = convert_from_reference (build_dynamic_cast_1 (type, expr, complain));
+  if (r != error_mark_node)
+    maybe_warn_about_useless_cast (type, expr, complain);
+  return r;
 }
-\f
+
 /* Return the runtime bit mask encoding the qualifiers of TYPE.  */
 
 static int
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 185473)
+++ cp/pt.c	(working copy)
@@ -13444,7 +13444,7 @@ tsubst_copy_and_build (tree t,
     case STATIC_CAST_EXPR:
       {
 	tree type;
-	tree op;
+	tree op, r = NULL_TREE;
 
 	type = tsubst (TREE_TYPE (t), args, complain, in_decl);
 	if (integral_constant_expression_p
@@ -13458,21 +13458,30 @@ tsubst_copy_and_build (tree t,
 
 	op = RECUR (TREE_OPERAND (t, 0));
 
+	++c_inhibit_evaluation_warnings;
 	switch (TREE_CODE (t))
 	  {
 	  case CAST_EXPR:
-	    return build_functional_cast (type, op, complain);
+	    r = build_functional_cast (type, op, complain);
+	    break;
 	  case REINTERPRET_CAST_EXPR:
-	    return build_reinterpret_cast (type, op, complain);
+	    r = build_reinterpret_cast (type, op, complain);
+	    break;
 	  case CONST_CAST_EXPR:
-	    return build_const_cast (type, op, complain);
+	    r = build_const_cast (type, op, complain);
+	    break;
 	  case DYNAMIC_CAST_EXPR:
-	    return build_dynamic_cast (type, op, complain);
+	    r = build_dynamic_cast (type, op, complain);
+	    break;
 	  case STATIC_CAST_EXPR:
-	    return build_static_cast (type, op, complain);
+	    r = build_static_cast (type, op, complain);
+	    break;
 	  default:
 	    gcc_unreachable ();
 	  }
+	--c_inhibit_evaluation_warnings;
+
+	return r;
       }
 
     case POSTDECREMENT_EXPR:
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 185473)
+++ cp/cp-tree.h	(working copy)
@@ -5860,6 +5860,7 @@ extern int lvalue_or_else			(tree, enum lvalue_use
 extern void check_template_keyword		(tree);
 extern bool check_raw_literal_operator		(const_tree decl);
 extern bool check_literal_operator_args		(const_tree, bool *, bool *);
+extern void maybe_warn_about_useless_cast       (tree, tree, tsubst_flags_t);
 
 /* in typeck2.c */
 extern void require_complete_eh_spec_types	(tree, tree);

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-03-19 17:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-16 21:45 [C++ Patch] PR 14710 (add -Wuseless-cast) Paolo Carlini
2012-03-17 16:04 ` Jason Merrill
2012-03-18 15:35   ` Paolo Carlini
2012-03-19 13:17     ` Jason Merrill
2012-03-19 13:57       ` Paolo Carlini
2012-03-19 14:59         ` Jason Merrill
2012-03-19 17:46           ` Paolo Carlini

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).