From patchwork Fri Jun 25 23:29:53 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Le-Chun Wu X-Patchwork-Id: 57031 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 B5BF0100918 for ; Sat, 26 Jun 2010 09:30:12 +1000 (EST) Received: (qmail 12189 invoked by alias); 25 Jun 2010 23:30:09 -0000 Received: (qmail 12170 invoked by uid 22791); 25 Jun 2010 23:30:04 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, SPF_HELO_PASS, TW_CX, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 25 Jun 2010 23:29:58 +0000 Received: from kpbe17.cbf.corp.google.com (kpbe17.cbf.corp.google.com [172.25.105.81]) by smtp-out.google.com with ESMTP id o5PNTt7h010507 for ; Fri, 25 Jun 2010 16:29:55 -0700 Received: from yxd5 (yxd5.prod.google.com [10.190.1.197]) by kpbe17.cbf.corp.google.com with ESMTP id o5PNTs9G028044 for ; Fri, 25 Jun 2010 16:29:54 -0700 Received: by yxd5 with SMTP id 5so73594yxd.19 for ; Fri, 25 Jun 2010 16:29:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.100.244.40 with SMTP id r40mr1819844anh.132.1277508593946; Fri, 25 Jun 2010 16:29:53 -0700 (PDT) Received: by 10.101.80.17 with HTTP; Fri, 25 Jun 2010 16:29:53 -0700 (PDT) Date: Fri, 25 Jun 2010 16:29:53 -0700 Message-ID: Subject: [patch][C++] Enhancements to -Wconversion-null From: Le-Chun Wu To: =?ISO-8859-1?Q?Manuel_L=F3pez=2DIb=E1=F1ez?= , Jason Merrill , GCC Patches 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 Hi, This patch addresses two issues in the current implementation of -Wconversion-null warning: 1. The current implementation warns on converting false to NULL for function arguments, but not on other expressions/statements. That is, it currently warns on void func1(int* ptr); void func2() { func1(false); } but not on int* t = false; This patch fixes this false-negative. 2. For templatized function calls, the current implementation will warn even if a NULL passed in as a function parameter is used to instantiate the template, which doesn't look right. For example, if we have this template function: template void func(T x) {} we want to warn in this call as the user is clearly passing a NULL for an integer parameter: void foo() { func(NULL); } but we don't want to warn in this call because the user clearly wants to pass in a NULL: void foo() { func(NULL); } However, the current implementation warns on both cases. Basically if a function argument is NULL and is used to implicitly instantiate a template function (and therefore bind the type of the corresponding parameter to 'long int', which is the type of NULL in C++), we don't want to warn about passing NULL to non-pointer argument. This patch addresses this issue. Bootstrapped and tested on x86_64-linux-gnu. OK for trunk? Thanks, Le-chun 2010-06-25 Le-Chun Wu gcc/cp/ChangeLog * call.c (build_new_function_call): Modify to track if a call has explicit template arguments. (conversion_null_warnings): Warn on converting false to NULL for non-function calls as well. (build_over_call): Do not warn on implicit instantiation with NULL. (build_new_method_call): Modify to track if a call has explicit template arguments. * cp-tree.h: Add a new flag LOOKUP_EXPLICIT_TMPL_ARGS. gcc/testsuite/ChangeLog * g++.dg/warn/Wconversion-null-2.C: Change the test as we no long emit warnings on implicit instantiation with NULL. * g++.dg/warn/Wconversion-null-3.C: New test. * g++.dg/warn/Wconversion-null-4.C: New test. * g++.dg/warn/Wconversion-null.C: Change comments (no warning on implicit instantiation with NULL). * g++.old-deja/g++.other/null3.C: Change the test as we start to warn on converting false to NULL in assignment. Index: gcc/cp/call.c =================================================================== --- gcc/cp/call.c (revision 161298) +++ gcc/cp/call.c (working copy) @@ -3240,7 +3240,16 @@ build_new_function_call (tree fn, VEC(tr result = error_mark_node; } else - result = build_over_call (cand, LOOKUP_NORMAL, complain); + { + int flags = LOOKUP_NORMAL; + /* If fn is template_id_expr, the call has explicit template arguments + (e.g. func(5)), communicate this info to build_over_call + through FLAGS so that later we can use it to decide whether to warn + about peculiar null pointer conversion. */ + if (TREE_CODE (fn) == TEMPLATE_ID_EXPR) + flags |= LOOKUP_EXPLICIT_TMPL_ARGS; + result = build_over_call (cand, flags, complain); + } /* Free all the conversions we allocated. */ obstack_free (&conversion_obstack, p); @@ -4870,10 +4879,16 @@ conversion_null_warnings (tree totype, t } /* Issue warnings if "false" is converted to a NULL pointer */ - else if (expr == boolean_false_node && fn && POINTER_TYPE_P (t)) - warning_at (input_location, OPT_Wconversion_null, - "converting % to pointer type for argument %P of %qD", - argnum, fn); + else if (expr == boolean_false_node && POINTER_TYPE_P (t)) + { + if (fn) + warning_at (input_location, OPT_Wconversion_null, + "converting % to pointer type for argument %P of %qD", + argnum, fn); + else + warning_at (input_location, OPT_Wconversion_null, + "converting % to pointer type"); + } } /* Perform the conversions in CONVS on the expression EXPR. FN and @@ -5722,6 +5737,7 @@ build_over_call (struct z_candidate *can { tree type = TREE_VALUE (parm); tree arg = VEC_index (tree, args, arg_index); + bool conversion_warning = true; conv = convs[i]; @@ -5731,6 +5747,32 @@ build_over_call (struct z_candidate *can && !TREE_ADDRESSABLE (type)) conv = conv->u.next; + /* If the argument is NULL and used to (implicitly) instantiate a + template function (and bind one of the template arguments to + the type of 'long int'), we don't want to warn about passing NULL + to non-pointer argument. + For example, if we have this template function: + + template void func(T x) {} + + we want to warn (with -Wconversion-null) in this case: + + void foo() { + func(NULL); + } + + but not in this case: + + void foo() { + func(NULL); + } + */ + if (arg == null_node + && DECL_TEMPLATE_INFO (fn) + && cand->template_decl + && !(flags & LOOKUP_EXPLICIT_TMPL_ARGS)) + conversion_warning = false; + /* Warn about initializer_list deduction that isn't currently in the working draft. */ if (cxx_dialect > cxx98 @@ -5761,7 +5803,10 @@ build_over_call (struct z_candidate *can } } - val = convert_like_with_context (conv, arg, fn, i-is_method, complain); + val = convert_like_with_context (conv, arg, fn, i-is_method, + conversion_warning + ? complain + : complain & (~tf_warning)); val = convert_for_arg_passing (type, val); if (val == error_mark_node) @@ -6501,6 +6546,12 @@ build_new_method_call (tree instance, tr if (DECL_VINDEX (fn) && ! (flags & LOOKUP_NONVIRTUAL) && resolves_to_fixed_type_p (instance, 0)) flags |= LOOKUP_NONVIRTUAL; + /* If this call has explicit template arguments, communicate + this info to build_over_call through FLAGS so that later + we can use it to decide whether to warn about peculiar null + pointer conversion. */ + if (explicit_targs) + flags |= LOOKUP_EXPLICIT_TMPL_ARGS; /* Now we know what function is being called. */ if (fn_p) *fn_p = fn; Index: gcc/cp/cp-tree.h =================================================================== --- gcc/cp/cp-tree.h (revision 161298) +++ gcc/cp/cp-tree.h (working copy) @@ -4168,6 +4168,8 @@ enum overload_flags { NO_SPECIAL = 0, DT #define LOOKUP_COPY_PARM (LOOKUP_NO_COPY_CTOR_CONVERSION << 1) /* We only want to consider list constructors. */ #define LOOKUP_LIST_ONLY (LOOKUP_COPY_PARM << 1) +/* An instantiation with explicit template arguments. */ +#define LOOKUP_EXPLICIT_TMPL_ARGS (LOOKUP_LIST_ONLY << 1) #define LOOKUP_NAMESPACES_ONLY(F) \ (((F) & LOOKUP_PREFER_NAMESPACES) && !((F) & LOOKUP_PREFER_TYPES)) Index: gcc/testsuite/g++.old-deja/g++.other/null3.C =================================================================== --- gcc/testsuite/g++.old-deja/g++.other/null3.C (revision 161298) +++ gcc/testsuite/g++.old-deja/g++.other/null3.C (working copy) @@ -2,5 +2,5 @@ void x() { - int* p = 1==0; + int* p = 1==0; // { dg-warning "converting 'false' to pointer" } } Index: gcc/testsuite/g++.dg/warn/Wconversion-null-2.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wconversion-null-2.C (revision 161298) +++ gcc/testsuite/g++.dg/warn/Wconversion-null-2.C (working copy) @@ -44,6 +44,6 @@ int main() k(NULL); // { dg-warning "" } converting NULL to int g(NULL); // { dg-warning "" } converting NULL to int h(); // No warning: NULL bound to integer template parameter - l(NULL); // { dg-warning "" } converting NULL to int + l(NULL); // No warning: NULL implicitly instantiated and bound to long NULL && NULL; // No warning: converting NULL to bool is OK } Index: gcc/testsuite/g++.dg/warn/Wconversion-null-3.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wconversion-null-3.C (revision 0) +++ gcc/testsuite/g++.dg/warn/Wconversion-null-3.C (revision 0) @@ -0,0 +1,15 @@ +// { dg-do compile } +// { dg-options "" } + +#include + +void func1(int* ptr); + +void func2() { + int* t = false; // { dg-warning "converting 'false' to pointer" } + int* p; + p = false; // { dg-warning "converting 'false' to pointer" } + int* r = sizeof(char) / 2; + func1(false); // { dg-warning "converting 'false' to pointer" } + int i = NULL; // { dg-warning "converting to non-pointer" } +} Index: gcc/testsuite/g++.dg/warn/Wconversion-null-4.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wconversion-null-4.C (revision 0) +++ gcc/testsuite/g++.dg/warn/Wconversion-null-4.C (revision 0) @@ -0,0 +1,45 @@ +// { dg-do compile } +// { dg-options "" } + +#include + +class Foo { + public: + template + static void Compare(const T1& expected, const T2& actual) { } + + template + static void Compare(const T1& expected, T2* actual) { } + +}; + +template +class Foo2 { + public: + Foo2(int x); + template void Bar(T2 y); +}; + +template void func(T3 x) { } + +typedef Foo2 MyFooType; + +void func1(long int a) { + MyFooType *foo2 = new MyFooType(NULL); // { dg-warning "passing NULL to" } + foo2->Bar(a); + func(NULL); // Shoud not warn + func(NULL); // { dg-warning "passing NULL to" } + func(NULL); +} + +int x = 1; + +main() +{ + int *p = &x; + + Foo::Compare(0, *p); + Foo::Compare(NULL, p); // { dg-warning "passing NULL to" } + Foo::Compare(NULL, p); // should not warn + func1(NULL); // { dg-warning "passing NULL to" } +} Index: gcc/testsuite/g++.dg/warn/Wconversion-null.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wconversion-null.C (revision 161298) +++ gcc/testsuite/g++.dg/warn/Wconversion-null.C (working copy) @@ -44,6 +44,6 @@ int main() k(NULL); // converting NULL to int g(NULL); // converting NULL to int h(); // No warning: NULL bound to integer template parameter - l(NULL); // converting NULL to int + l(NULL); // No warning: NULL implicitly instantiated and bound to long NULL && NULL; // No warning: converting NULL to bool is OK }