From patchwork Fri May 29 17:39:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 478004 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49B80140F91 for ; Sat, 30 May 2015 03:39:42 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=YsxEkpyn; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=wki2MqTAzZpI/s/5 rcjfWRjX9uJunu74V1LKwpUlmo+xwN/Ltr0n95USfgAuzyhMNsm9WrWYj3Sqkp1d ru2ZYIh0RY0YFGalmoesxpfeF3Rr0yKfLx2Me3s7+QyXMh1nBxSg8mZ0p29BaSVJ wHtQZ3SaUMhT+bhVcyOPSinpwbk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=JvRjPaY5kTjuEDsxAS3kqv 6V9uQ=; b=YsxEkpynMZvbEpZXSkvbNyZBzNHZ/BOSJbl6o0aU00eTipBrTzJI4j bm3/Xbr202syfPh9xAex4NTe17HNv2nq0zxP5PX43VTHsCMEG7FUmEoRYOBWMLzx 39Q5SBt61fPMSBokK7LEngN6TKeC5pFMKqMgSueXhDeMsBxvzsf7c= Received: (qmail 112842 invoked by alias); 29 May 2015 17:39:35 -0000 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 Received: (qmail 112832 invoked by uid 89); 29 May 2015 17:39:34 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=AWL, BAYES_50, KAM_ASCII_DIVIDERS, SPF_PASS autolearn=no version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 29 May 2015 17:39:31 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by uk-mta-17.uk.mimecast.lan; Fri, 29 May 2015 18:39:28 +0100 Received: from localhost ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 29 May 2015 18:39:28 +0100 From: Richard Sandiford To: gcc-patches@gcc.gnu.org Mail-Followup-To: gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Subject: RFC: Add C++ warnings for unnecessarily general return types Date: Fri, 29 May 2015 18:39:28 +0100 Message-ID: <87lhg7l1v3.fsf@e105548-lin.cambridge.arm.com> User-Agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-MC-Unique: EKIeM19zS6qppeFRQkIyzA-1 One of the main early aims of the rtl refactoring is to finally separate instructions (rtx_insns) from other rtxes. I thought it would help if the compiler could warn about cases where a function returns rtx when it could return rtx_insn*, or where a variable has type rtx but could have type rtx_insn*. This patch tries to implement something along those lines. There are two warning options, one for function returns and one for variables. There are obviously many good reasons why a functon or variable might have a more general type than it might appear to need, so the options certainly aren't -Wall or -Wextra material. But it might be worth having them as stage 2 and 3 warnings. I have a local patch that fixes all instances of the warnings in an x86_64-linux-gnu bootstrap. This is very much an RFC. Maybe the options are too special-purpose to be worth including. Or, if they are worth including, there are probably interesting corner cases that I've not thought about. The option did find a couple of useful things besides rtx->rtx_insn* though. For example, in ipa-comdats.c:enqueue_references we have: symtab_node *node = edge->callee->ultimate_alias_target (); /* Always keep thunks in same sections as target function. */ if (is_a (node)) node = dyn_cast (node)->function_symbol (); if (!node->aux && node->definition) { node->aux = *first; *first = node; } ultimate_alias_target always returns a cgraph_node*, so the is_a is always true. There are also two cases where we have unnecessary safe_as_a s. Thanks, Richard gcc/ * doc/invoke.texi (-Wupcast-result, -Wupcast-var): Document. * doc/extend.texi (no_upcast_warning): Document. gcc/c-family/ * c.opt (Wupcast-result, Wupcast-var): New options. * c-common.c (c_common_attribute_table): Add no_upcast_warning. (handle_no_upcast_warning_attribute): New function. gcc/cp/ * cp-tree.h (cp_decl_target_types): New structure. (cp_decl_target_types_hasher): Likewise. (language_function): Add x_result_target_types and x_decl_target_types. (current_function_return_target_types): New macro. (current_function_decl_target_types): Likewise. (upcast_var_record_type): Declare. * decl.c (get_common_target, get_upcast_warning_type): New functions. (poplevel): Handle warn_upcast_var. (finish_function): Handle warn_upcast_result. Clear out new language_function fields. * typeck.c (cp_decl_target_types_hasher::hash): New function. (cp_decl_target_types_hasher::equal): Likewise. (joust_upcast_types, upcast_var_record_type): Likewise. (cp_build_modify_expr): Record upcasts for warn_upcast_var. (check_return_expr): Likewise warn_upcast_result. * typeck2.c (store_init_value): Record upcasts for warn_upcast_var. gcc/testsuite/ * g++.dg/warn/Wupcast-result-1.C, g++.dg/warn/Wupcast-result-2.C, g++.dg/warn/Wupcast-result-3.C, g++.dg/warn/Wupcast-result-4.C, g++.dg/warn/Wupcast-var-1.C: New tests. Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi 2015-05-27 14:37:58.553464723 +0100 +++ gcc/doc/invoke.texi 2015-05-27 15:44:05.624385485 +0100 @@ -288,6 +288,7 @@ 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 +-Wupcast-result -Wupcast-var @gol -Wuseless-cast -Wvariadic-macros -Wvector-operation-performance @gol -Wvla -Wvolatile-register-var -Wwrite-strings @gol -Wzero-as-null-pointer-constant} @@ -4879,6 +4880,43 @@ compilations. Warn when deleting a pointer to incomplete type, which may cause undefined behavior at runtime. This warning is enabled by default. +@item -Wupcast-result @r{(C++ and Objective-C++ only)} +@opindex Wupcast-result +@opindex Wno-upcast-result +Warn about functions whose return type is more general than it needs +to be. Specifically, if a function returns a pointer or reference +to some base class @var{B} and all nonnull returns have an implicit +upcast from a derived class, warn if there is a derived class @var{D} +that would satisfy all such returns. + +The warning does not consider @var{D} to be a candidate if it is in an +anonymous namespace or if it has the @code{no_upcast_warning} attribute. +It also ignores returns from virtual functions, where general types are +likely to be deliberate. + +For example, the warning would trigger on the following function, +which could return @code{D1*} rather than @code{B*}: +@cindex @code{no_upcast_warning} type attribute +@smallexample +struct B @{@}; +struct D1 : public B @{@}; +struct D2 : public D1 @{@}; +B *f(int x) @{ if (x) return new D1; else return new D2; @} +@end smallexample + +@item -Wupcast-var @r{(C++ and Objective-C++ only)} +@opindex Wupcast-var +@opindex Wno-upcast-var +Like @option{-Wupcast-result}, but warn about local variables that are +more general than they need to be. For example, the warning would trigger +on @code{x} in the following function, which could have type @code{D1*} +rather than @code{B}: +@smallexample +struct B @{@}; +struct D1 : public B @{@}; +void f() @{ B *x = new D1; @} +@end smallexample + @item -Wuseless-cast @r{(C++ and Objective-C++ only)} @opindex Wuseless-cast @opindex Wno-useless-cast Index: gcc/doc/extend.texi =================================================================== --- gcc/doc/extend.texi 2015-05-26 12:00:59.000225870 +0100 +++ gcc/doc/extend.texi 2015-05-27 15:45:25.975471274 +0100 @@ -6179,6 +6179,19 @@ the case with lock or thread classes, wh not referenced, but contain constructors and destructors that have nontrivial bookkeeping functions. +@item no_upcast_warning +@cindex @code{no_upcast_warning} type attribute +Specifies that a struct or class should be ignored by the C++ options +@option{-Wupcast-result} and @option{-Wupcast-var} when searching for +a more derived class. For example: +@smallexample +struct B @{@}; +struct __attribute__((no_upcast_warning)) D1 : public B @{@}; +B *f(int x) @{ return new D1; @} +@end smallexample +would not trigger a warning even when @option{-Wupcast-result} is +in effect. + @item visibility @cindex @code{visibility} type attribute In C++, attribute visibility (@pxref{Function Attributes}) can also be @@ -6192,7 +6205,6 @@ particular, if a class is thrown as an e and caught in another, the class must have default visibility. Otherwise the two shared objects are unable to use the same typeinfo node and exception handling will break. - @end table To specify multiple attributes, separate them by commas within the Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt 2015-05-27 14:37:58.553464723 +0100 +++ gcc/c-family/c.opt 2015-05-27 14:37:58.541464861 +0100 @@ -896,6 +896,14 @@ Wunused-result C ObjC C++ ObjC++ Var(warn_unused_result) Init(1) Warning Warn if a caller of a function, marked with attribute warn_unused_result, does not use its return value +Wupcast-result +C++ ObjC++ Var(warn_upcast_result) Warning +Warn when every nonnull return from a function performs an upcast from the same derived type to a base type. + +Wupcast-var +C++ ObjC++ Var(warn_upcast_var) Warning +Warn when every nonnull assignment to a variable performs an upcast from the same derived type to a base type. + Wvariadic-macros C ObjC C++ ObjC++ CPP(warn_variadic_macros) CppReason(CPP_W_VARIADIC_MACROS) Var(cpp_warn_variadic_macros) Init(0) Warning LangEnabledBy(C ObjC C++ ObjC++,Wpedantic || Wtraditional) Warn about using variadic macros Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c 2015-05-27 14:37:58.553464723 +0100 +++ gcc/c-family/c-common.c 2015-05-27 14:37:58.541464861 +0100 @@ -404,6 +404,8 @@ static tree handle_designated_init_attri static tree handle_bnd_variable_size_attribute (tree *, tree, tree, int, bool *); static tree handle_bnd_legacy (tree *, tree, tree, int, bool *); static tree handle_bnd_instrument (tree *, tree, tree, int, bool *); +static tree handle_no_upcast_warning_attribute (tree *, tree, tree, int, + bool *); static void check_function_nonnull (tree, int, tree *); static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT); @@ -823,6 +825,8 @@ const struct attribute_spec c_common_att handle_bnd_legacy, false }, { "bnd_instrument", 0, 0, true, false, false, handle_bnd_instrument, false }, + { "no_upcast_warning", 0, 0, false, true, false, + handle_no_upcast_warning_attribute, false }, { NULL, 0, 0, false, false, false, NULL, false } }; @@ -8718,6 +8722,22 @@ handle_bnd_instrument (tree *node, tree { warning (OPT_Wattributes, "%qE attribute ignored", name); *no_add_attrs = true; + } + + return NULL_TREE; +} + +/* Handle a "no_upcast_warning" attribute; arguments as in + struct attribute_spec.handler. */ + +static tree +handle_no_upcast_warning_attribute (tree *node, tree name, tree, int, + bool *no_add_attrs) +{ + if (TREE_CODE (*node) != RECORD_TYPE) + { + warning (OPT_Wattributes, "%qE attribute ignored", name); + *no_add_attrs = true; } return NULL_TREE; Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h 2015-05-27 14:37:58.553464723 +0100 +++ gcc/cp/cp-tree.h 2015-05-27 15:26:56.200086947 +0100 @@ -1200,6 +1200,23 @@ struct named_label_hasher : ggc_hasher *target_types; +}; + +struct cp_decl_target_types_hasher : ggc_hasher +{ + static hashval_t hash (cp_decl_target_types *); + static bool equal (cp_decl_target_types *, cp_decl_target_types *); +}; + /* Global state pertinent to the current function. */ struct GTY(()) language_function { @@ -1232,6 +1249,8 @@ struct GTY(()) language_function { /* Tracking possibly infinite loops. This is a vec only because vec doesn't work with gtype. */ vec *infinite_loops; + vec *x_result_target_types; + hash_table *x_decl_target_types; hash_table *extern_decl_map; }; @@ -1307,6 +1326,20 @@ #define in_function_try_handler cp_funct #define current_function_return_value \ (cp_function_chain->x_return_value) +/* Used to implement -Wupcast-result. If the current function returns a + pointer or reference, this is the list of types with which the + pointer or reference target (X) must be compatible. Each one is either + X or is derived from X. */ + +#define current_function_result_target_types \ + (cp_function_chain->x_result_target_types) + +/* Used to implement -Wupcast-var. It is a hash table of all variables + in the current function that might be relevant to the warning. */ + +#define current_function_decl_target_types \ + (cp_function_chain->x_decl_target_types) + /* A type involving 'auto' to be used for return type deduction. */ #define current_function_auto_return_pattern \ @@ -6241,6 +6274,7 @@ extern tree cp_build_c_cast (tree, tre extern tree build_x_modify_expr (location_t, tree, enum tree_code, tree, tsubst_flags_t); +extern void upcast_var_record_type (tree, tree); extern tree cp_build_modify_expr (tree, enum tree_code, tree, tsubst_flags_t); extern tree convert_for_initialization (tree, tree, tree, int, Index: gcc/cp/decl.c =================================================================== --- gcc/cp/decl.c 2015-05-27 14:37:58.553464723 +0100 +++ gcc/cp/decl.c 2015-05-27 15:32:12.872489637 +0100 @@ -278,6 +278,76 @@ typedef struct GTY(()) incomplete_var_d static GTY(()) vec *incomplete_vars; +/* On entry: + + - Each element of TARGETS is, or is derived from, BASE. + - TARGETS[0] is, or is derived from, CANDIDATE; and + - CANDIDATE is, or is derived from, BASE + + Find and return a CANDIDATE that satisfies the additional properties: + + - Each element of TARGETS is, or is derived from, CANDIDATE + - CANDIDATE does not have the no_upcast_warning attribute + + In hierarchies with no virtual bases, always return the most derived + such type (effectively the maximum lower bound). In hierarchies with + virtual bases the choice is more arbitrary, but is never BASE if an + alternative exists. */ + +static tree +get_common_target (vec &targets, tree base, tree candidate) +{ + if (!lookup_attribute ("no_upcast_warning", TYPE_ATTRIBUTES (candidate))) + { + /* We already know CANDIDATE satisfies the condition for TARGETS[0]. */ + unsigned int i; + for (i = 1; i < targets.length (); ++i) + { + tree other = targets[i]; + if (candidate != other && !DERIVED_FROM_P (candidate, other)) + break; + } + if (i == targets.length ()) + return candidate; + } + tree binfo = TYPE_BINFO (candidate); + tree next_binfo; + for (unsigned int i = 0; BINFO_BASE_ITERATE (binfo, i, next_binfo); ++i) + { + tree next_try = BINFO_TYPE (next_binfo); + if (DERIVED_FROM_P (base, next_try)) + { + tree res = get_common_target (targets, base, next_try); + if (res != base) + return res; + } + } + return base; +} + +/* LHS_TYPE is a pointer or reference type and TARGETS is a list of + types that are, or are derived from, TREE_TYPE (LHS_TYPE). If the + same condition would hold for a derived class of TREE_TYPE (LHS_TYPE), + pick one such type -- as for get_common_target -- and return a pointer + or reference like LHS_TYPE that has that target. Return null otherwise. */ + +static tree +get_upcast_warning_type (vec &targets, tree lhs_type) +{ + tree lhs_target = TYPE_MAIN_VARIANT (TREE_TYPE (lhs_type)); + tree alt_target = get_common_target (targets, lhs_target, targets[0]); + if (alt_target == lhs_target + || decl_anon_ns_mem_p (alt_target)) + return NULL_TREE; + + alt_target = cp_build_qualified_type + (alt_target, cp_type_quals (TREE_TYPE (lhs_type))); + if (TREE_CODE (lhs_type) == POINTER_TYPE) + return build_pointer_type (alt_target); + else + return build_reference_type (alt_target); +} + /* Returns the kind of template specialization we are currently processing, given that it's declaration contained N_CLASS_SCOPES explicit scope qualifications. */ @@ -633,8 +703,13 @@ poplevel (int keep, int reverse, int fun leaving_for_scope = current_binding_level->kind == sk_for && flag_new_for_scope == 1; - /* Before we remove the declarations first check for unused variables. */ - if ((warn_unused_variable || warn_unused_but_set_variable) + /* Before we remove the declarations first check for unused variables + and variables with lax types. */ + if ((warn_unused_variable + || warn_unused_but_set_variable + || (warn_upcast_var + && cfun + && current_function_decl_target_types)) && current_binding_level->kind != sk_template_parms && !processing_template_decl) for (tree d = getdecls (); d; d = TREE_CHAIN (d)) @@ -644,7 +719,8 @@ poplevel (int keep, int reverse, int fun getdecls is built. */ decl = TREE_CODE (d) == TREE_LIST ? TREE_VALUE (d) : d; tree type = TREE_TYPE (decl); - if (VAR_P (decl) + if ((warn_unused_variable || warn_unused_but_set_variable) + && VAR_P (decl) && (! TREE_USED (decl) || !DECL_READ_P (decl)) && ! DECL_IN_SYSTEM_HEADER (decl) && DECL_NAME (decl) && ! DECL_ARTIFICIAL (decl) @@ -666,6 +742,25 @@ poplevel (int keep, int reverse, int fun unused_but_set_errorcount = errorcount; } } + if (VAR_P (decl) + && !TREE_ADDRESSABLE (decl) + && warn_upcast_var + && cfun + && current_function_decl_target_types) + { + cp_decl_target_types dummy = { decl, NULL }; + cp_decl_target_types *entry + = current_function_decl_target_types->find (&dummy); + if (entry && !vec_safe_is_empty (entry->target_types)) + { + tree var_type = TREE_TYPE (decl); + tree alt_type = get_upcast_warning_type (*entry->target_types, + var_type); + if (alt_type) + warning (OPT_Wupcast_var, "%q+D could have type %qT" + " instead of %qT", decl, alt_type, var_type); + } + } } /* Remove declarations for all the DECLs in this level. */ @@ -14294,6 +14389,19 @@ finish_function (int flags) TREE_NO_WARNING (fndecl) = 1; } + if (!DECL_VIRTUAL_P (fndecl) + && !vec_safe_is_empty (current_function_result_target_types) + && warn_upcast_result) + { + tree ret_type = TREE_TYPE (fntype); + tree alt_type = get_upcast_warning_type + (*current_function_result_target_types, ret_type); + if (alt_type) + warning_at (DECL_SOURCE_LOCATION (fndecl), OPT_Wupcast_result, + "function could return %qT instead of %qT", + alt_type, ret_type); + } + /* Store the end of the function, so that we get good line number info for the epilogue. */ cfun->function_end_locus = input_location; @@ -14345,6 +14453,8 @@ finish_function (int flags) f->bindings = NULL; f->extern_decl_map = NULL; f->infinite_loops = NULL; + f->x_result_target_types = NULL; + f->x_decl_target_types = NULL; } /* Clear out the bits we don't need. */ local_names = NULL; Index: gcc/cp/typeck.c =================================================================== --- gcc/cp/typeck.c 2015-05-27 14:37:58.553464723 +0100 +++ gcc/cp/typeck.c 2015-05-27 15:29:37.190258405 +0100 @@ -74,6 +74,77 @@ static void warn_args_num (location_t, t static int convert_arguments (tree, vec **, tree, int, tsubst_flags_t); +hashval_t +cp_decl_target_types_hasher::hash (cp_decl_target_types *entry) +{ + return DECL_UID (entry->var); +} + +bool +cp_decl_target_types_hasher::equal (cp_decl_target_types *a, + cp_decl_target_types *b) +{ + return a->var == b->var; +} + +/* RHS is being assigned to a variable of type LHS_TYPE or is being returned + from a function that returns LHS_TYPE. If LHS_TYPE is a pointer to a class, + update TARGET_TYPES so that it would be safe to replace TREE_TYPE (LHS_TYPE) + with some type X if each member of TARGET_TYPES is, or is derived + from, X. */ + +static void +joust_upcast_types (vec *&target_types, tree lhs_type, tree rhs) +{ + /* Only consider pointers to classes and references to classes. */ + if (!POINTER_TYPE_P (lhs_type)) + return; + tree lhs_target = TYPE_MAIN_VARIANT (TREE_TYPE (lhs_type)); + if (!CLASS_TYPE_P (lhs_target)) + return; + + /* Ignore null values, since they could come from type-specific + macros (like GCC's NULL_RTX). */ + if (null_ptr_cst_p (rhs) || integer_zerop (rhs)) + return; + + tree rhs_type = TREE_TYPE (rhs); + if (TREE_CODE (lhs_type) == TREE_CODE (rhs_type) + || TREE_CODE (lhs_type) == REFERENCE_TYPE) + { + /* We're only interested in cases where the target of RHS_TYPE + is derived from the target of LHS_TYPE. */ + tree rhs_target = (TREE_CODE (lhs_type) == TREE_CODE (rhs_type) + ? TREE_TYPE (rhs_type) : rhs_type); + rhs_target = TYPE_MAIN_VARIANT (rhs_target); + if (CLASS_TYPE_P (rhs_target) + && lhs_target != rhs_target + && DERIVED_FROM_P (lhs_target, rhs_target)) + { + /* Do nothing if RHS_TARGET is derived from an existing type. */ + unsigned int i; + tree other_target; + FOR_EACH_VEC_SAFE_ELT (target_types, i, other_target) + if (other_target == rhs_target + || DERIVED_FROM_P (other_target, rhs_target)) + return; + + /* Remove any entries derived from RHS_TARGET. */ + unsigned int j = 0; + FOR_EACH_VEC_SAFE_ELT (target_types, i, other_target) + if (!DERIVED_FROM_P (rhs_target, other_target)) + (*target_types)[j++] = other_target; + + vec_safe_truncate (target_types, j); + vec_safe_push (target_types, rhs_target); + return; + } + } + /* The worst case: keep the original target. */ + vec_safe_truncate (target_types, 0); + vec_safe_push (target_types, lhs_target); +} + /* Do `exp = require_complete_type (exp);' to make sure exp does not have an incomplete type. (That includes void types.) Returns error_mark_node if the VALUE does not have @@ -7307,6 +7378,33 @@ build_modify_expr (location_t /*location return cp_build_modify_expr (lhs, modifycode, rhs, tf_warning_or_error); } +/* Record an assignment from RHS to LHS for -Wupcast-var. */ + +void +upcast_var_record_type (tree lhs, tree rhs) +{ + tree lhstype = TREE_TYPE (lhs); + if (VAR_P (lhs) + && !TREE_STATIC (lhs) + && DECL_CONTEXT (lhs) == current_function_decl + && POINTER_TYPE_P (lhstype) + && CLASS_TYPE_P (TREE_TYPE (lhstype))) + { + cp_decl_target_types dummy = { lhs, NULL }; + if (!current_function_decl_target_types) + current_function_decl_target_types + = hash_table::create_ggc (32); + cp_decl_target_types **slot + = current_function_decl_target_types->find_slot (&dummy, INSERT); + if (!*slot) + { + *slot = ggc_alloc (); + **slot = dummy; + } + joust_upcast_types ((*slot)->target_types, lhstype, rhs); + } +} + /* Build an assignment expression of lvalue LHS from value RHS. MODIFYCODE is the code for a binary operator that we use to combine the old value of LHS with RHS to get the new value. @@ -7427,6 +7525,9 @@ cp_build_modify_expr (tree lhs, enum tre break; } + if (warn_upcast_var) + upcast_var_record_type (lhs, rhs); + if (modifycode == INIT_EXPR) { if (BRACE_ENCLOSED_INITIALIZER_P (rhs)) @@ -8537,6 +8638,10 @@ check_return_expr (tree retval, bool *no functype = type; } + if (warn_upcast_result) + joust_upcast_types (current_function_result_target_types, + functype, retval); + /* When no explicit return-value is given in a function with a named return value, the named return value is used. */ result = DECL_RESULT (current_function_decl); Index: gcc/cp/typeck2.c =================================================================== --- gcc/cp/typeck2.c 2015-05-27 14:37:58.553464723 +0100 +++ gcc/cp/typeck2.c 2015-05-27 14:37:58.549464769 +0100 @@ -814,6 +814,9 @@ store_init_value (tree decl, tree init, value = extend_ref_init_temps (decl, value, cleanups); + if (warn_upcast_var) + upcast_var_record_type (decl, init); + /* In C++11 constant expression is a semantic, not syntactic, property. In C++98, make sure that what we thought was a constant expression at template definition time is still constant and otherwise perform this Index: gcc/testsuite/g++.dg/warn/Wupcast-result-1.C =================================================================== --- /dev/null 2015-05-18 19:38:59.338844008 +0100 +++ gcc/testsuite/g++.dg/warn/Wupcast-result-1.C 2015-05-27 14:37:58.549464769 +0100 @@ -0,0 +1,136 @@ +// { dg-options "-Wupcast-result -std=c++11" } + +struct B {}; +struct B1 : public B {}; +struct B11 : public B1 {}; +struct B12 : public B1 {}; +struct B121 : public B12 {}; +struct B2 : public B {}; +namespace { struct B3 : public B {}; } + +B * +f1 (int x) // { dg-warning "function could return 'B1\\*'" } +{ + if (x == 1) + return new B11; + if (x == 2) + return new B12; + if (x == 3) + return (B *) 0; + if (x == 4) + return nullptr; + if (x == 5) + return 0; + if (x == 6) + return (B1 *) 0; + if (x == 7) + return (B11 *) 0; + if (x == 8) + return (B12 *) 0; + return new B1; +} + +B * +f2 (int x) // { dg-warning "function could return 'B1\\*'" } +{ + if (x == 1) + return new B12; + if (x == 2) + return new B11; + return new B1; +} + +B * +f3 (int x) // { dg-warning "function could return 'B1\\*'" } +{ + if (x == 1) + return new B1; + if (x == 2) + return new B11; + return new B12; +} + +B * +f4 (int x) // { dg-warning "function could return 'B1\\*'" } +{ + if (x == 1) + return new B11; + return new B12; +} + +B * +f5 (int x) +{ + if (x == 1) + return new B1; + return new B2; +} + +B * +f6 (int x) +{ + if (x == 1) + return new B11; + return new B2; +} + +B * +f7 (int x) +{ + if (x == 1) + return new B12; + return new B2; +} + +B * +f8 (int x) // { dg-warning "function could return 'B12\\*'" } +{ + if (x == 1) + return new B12; + return new B121; +} + +B * +f9 () +{ + return (B *) new B1; +} + +B * +f10 () +{ + return new B3; +} + +B * +f11 () +{ + return new B; +} + +B * +f12 () +{ + return (B2 *) 0; +} + +int * +f13 () +{ + return new int; +} + +int +f14 () +{ + return 0; +} + +B ** +f15 (B **x) +{ + return x; +} + +struct X1 { B *foo () { return new B1; } }; // { dg-warning "function could return 'B1\\*'" } +struct X2 { virtual B *foo () { return new B1; } }; Index: gcc/testsuite/g++.dg/warn/Wupcast-result-2.C =================================================================== --- /dev/null 2015-05-18 19:38:59.338844008 +0100 +++ gcc/testsuite/g++.dg/warn/Wupcast-result-2.C 2015-05-27 14:37:58.549464769 +0100 @@ -0,0 +1,83 @@ +// { dg-options "-Wupcast-result" } + +struct B {}; +struct B1 : virtual public B {}; +struct B2 : virtual public B {}; +struct B3 : virtual public B {}; +struct B4 : virtual public B {}; +struct B1B2 : virtual public B1, virtual public B2 {}; +struct B1B2B3 : virtual public B1, virtual public B2, virtual public B3 {}; +struct B1B2B4 : virtual public B1, virtual public B2, virtual public B4 {}; +struct B2B3 : virtual public B2, virtual public B3 {}; +struct B3B4 : virtual public B3, virtual public B4 {}; + +B * +f1 (int x) // { dg-warning "function could return 'B1\\*'" } +{ + if (x == 1) + return new B1(); + return new B1B2(); +} + +B * +f2 (int x) // { dg-warning "function could return 'B2\\*'" } +{ + if (x == 1) + return new B2(); + return new B1B2(); +} + +B * +f3 (int x) // { dg-warning "function could return 'B2\\*'" } +{ + if (x == 1) + return new B1B2(); + if (x == 2) + return new B1B2B3(); + return new B2(); +} + +B * +f4 (int x) // { dg-warning "function could return 'B2\\*'" } +{ + if (x == 1) + return new B1B2B3(); + if (x == 2) + return new B1B2B4(); + return new B2(); +} + +const volatile B * +f5 (int x) // { dg-warning "function could return 'const volatile B1\\*'" } +{ + if (x == 1) + return new B1B2B3(); + if (x == 2) + return new B1B2B4(); + return new B1(); +} + +// Expect B1 to picked over B2, since it's listed first. +const B * +f6 (int x) // { dg-warning "function could return 'const B1\\*'" } +{ + if (x == 1) + return new B1B2; + return new B1B2B3; +} + +volatile B * +f7 (int x) // { dg-warning "function could return 'volatile B2\\*'" } +{ + if (x == 1) + return new B1B2; + return new B2B3; +} + +B * +f8 (int x) +{ + if (x == 1) + return new B1B2; + return new B3B4; +} Index: gcc/testsuite/g++.dg/warn/Wupcast-result-3.C =================================================================== --- /dev/null 2015-05-18 19:38:59.338844008 +0100 +++ gcc/testsuite/g++.dg/warn/Wupcast-result-3.C 2015-05-27 14:37:58.553464723 +0100 @@ -0,0 +1,31 @@ +// { dg-options "-Wupcast-result" } + +struct B {}; +struct B1 : public B {}; +struct B2 : public B {}; +struct B21 : public B2 {}; + +B2 b2; +const B2 cb2; +B21 b21; +const B21 cb21; + +const B & +f1 (int x) // { dg-warning "function could return 'const B2&'" } +{ + if (x == 1) + return cb2; + return cb21; +} + +B & +f2 (B1 &y) // { dg-warning "function could return 'B1&'" } +{ + return y; +} + +B & +f3 (B1 &y) +{ + return (B &) y; +} Index: gcc/testsuite/g++.dg/warn/Wupcast-result-4.C =================================================================== --- /dev/null 2015-05-18 19:38:59.338844008 +0100 +++ gcc/testsuite/g++.dg/warn/Wupcast-result-4.C 2015-05-27 14:37:58.553464723 +0100 @@ -0,0 +1,15 @@ +// { dg-options "-Wupcast-result" } + +struct B {}; +struct B1 : public B {}; +struct __attribute__((no_upcast_warning)) B11 : public B1 {}; +struct B111 : public B11 {}; +struct B112 : public B11 {}; + +B * +f1 (int x) // { dg-warning "function could return 'B1\\*'" } +{ + if (x == 1) + return new B111; + return new B112; +} Index: gcc/testsuite/g++.dg/warn/Wupcast-var-1.C =================================================================== --- /dev/null 2015-05-18 19:38:59.338844008 +0100 +++ gcc/testsuite/g++.dg/warn/Wupcast-var-1.C 2015-05-27 14:37:58.553464723 +0100 @@ -0,0 +1,160 @@ +// { dg-options "-Wupcast-var -std=c++11" } + +struct B {}; +struct B1 : public B {}; +struct B11 : public B1 {}; +struct B12 : public B1 {}; +struct B121 : public B12 {}; +struct B2 : public B {}; +namespace { struct B3 : public B {}; } + +void +f1 () +{ + B *x; // { dg-warning "could have type 'B1\\*'" } + x = new B11(); + x = new B12(); + x = (B *) 0; + x = nullptr; + x = 0; + x = (B1 *) 0; + x = (B11 *) 0; + x = (B12 *) 0; + x = new B1; +} + +void +f2 (void) +{ + B *x = new B12(); // { dg-warning "could have type 'B1\\*'" } + x = new B11(); + x = new B1(); +} + +void +f3 (void) +{ + B *x = new B1(); // { dg-warning "could have type 'B1\\*'" } + x = new B12(); + x = new B11(); +} + +void +f4 () +{ + B *x = new B11(); // { dg-warning "could have type 'B1\\*'" } + x = new B12(); +} + +void +f5 () +{ + B *x = new B1; + x = new B2; +} + +void +f6 () +{ + B *x = new B11; + x = new B2; +} + +void +f7 () +{ + B *x = new B12; + x = new B2; +} + +void +f8 () +{ + B *x = new B12(); // { dg-warning "could have type 'B12\\*'" } + x = new B121(); +} + +void +f9 () +{ + B *x = (B *) new B1; +} + +void +f10 () +{ + B *x = new B3; +} + +void +f11 () +{ + B *x = new B; +} + +void +f12 () +{ + B *x = (B2 *) 0; +} + +void +f13 () +{ + int *x = new int; +} + +void +f14 () +{ + int x = 0; +} + +void +f15 (B **y) +{ + B **x = y; +} + +void +f15 (B *y) +{ + y = new B1; +} + +void foo (B **x); + +void +f16 () +{ + B *x = new B11; + foo (&x); +} + +void +f17 () +{ + static B *x = new B11; +} + +void +f18 () +{ + static B *x = new B11; + foo (&x); +} + +void +f19 () +{ + extern B *x; + x = new B11; +} + +void +f20 () +{ + extern B *x; + x = new B11; + foo (&x); +}