From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 339A23858D29 for ; Wed, 22 Sep 2021 20:12:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 339A23858D29 Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-94-E-mw13l0MlKIWTchXYW2lw-1; Wed, 22 Sep 2021 16:12:57 -0400 X-MC-Unique: E-mw13l0MlKIWTchXYW2lw-1 Received: by mail-qk1-f198.google.com with SMTP id bi14-20020a05620a318e00b00432f0915dd6so12707897qkb.6 for ; Wed, 22 Sep 2021 13:12:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=NCX9l9zWEMOAvgPDdOwti0HKhF0rbVk5TuC2lZu738g=; b=VIbf+ZJ8s4+U1RMM6P60yl+su/iDhsST9yxWvPv2wB+9U9IGsISRI30MrEvb5l6dfn 9gM+v2NTFa3q4i/ZFVfpYczZr4UeHqgUOdAZ87cvujYbAAzRKA8b8FTgiTuvlXjVuchi rJK6HUl64J8MaNlApJkTa9YWy6Sm+iEdXPg3j2zCWms66y/0p9r5aNSoiuqMwwiqvwzD H1bZ+y6oEJWbFtSGmRixgLiUQjKT7LODTSD9xl0I56QAB3jWWp5ZBFpZkFk67Z5vnJv+ 7029LvFYjjf1EWMKHkbFQdb4lAYgaizyZOAPm7Yo22BbV21hjslpARPV83jGebJ5rlrN xIFg== X-Gm-Message-State: AOAM530Tjkb4iZ26S4ZPE7ujZRje8UYgr+cA5LYV8vyzF7HXPZP4woHX KS4p7Sd3DD/Di+AKItmVDKFgc4wkKHm4RAlumh918hUgB1eM5QI0t2auvsTsDbIuI//QyNWEz8n ETMm/YcuY5OBn0b5WXg== X-Received: by 2002:a0c:c18e:: with SMTP id n14mr1102210qvh.60.1632341577349; Wed, 22 Sep 2021 13:12:57 -0700 (PDT) X-Google-Smtp-Source: ABdhPJymrd+/JiOgvDJuchy0DL3caPbrclFx3mDn2GX8JkqUL12DmXrydFL4lvTmKMPTRA8iaflAVA== X-Received: by 2002:a0c:c18e:: with SMTP id n14mr1102167qvh.60.1632341576985; Wed, 22 Sep 2021 13:12:56 -0700 (PDT) Received: from [192.168.1.149] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id z12sm2135528qti.58.2021.09.22.13.12.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Sep 2021 13:12:56 -0700 (PDT) Message-ID: <17af67fd-6b62-ec28-abe7-f25e16bde2aa@redhat.com> Date: Wed, 22 Sep 2021 16:12:55 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.1 Subject: Re: [PATCH] warn for more impossible null pointer tests [PR102103] To: Martin Sebor , gcc-patches , "Joseph S. Myers" , Jeff Law References: <0858e35e-3271-0dc2-4376-51228645ce79@redhat.com> <40945df3-b63d-87f5-2b9b-e926a24bb8b5@redhat.com> <0f38b998-7894-b8c6-80ff-a822848c9b0b@gmail.com> <8a41e370-7962-7374-f302-8930f8ebeb47@redhat.com> <4ecd02d0-2053-824f-cc42-ced0070f57c9@gmail.com> <037b1948-5467-7333-5a9f-0e7ae75529e9@gmail.com> <57b90a41-a952-80d0-4c3e-a5654415e741@redhat.com> <8526359b-bbd7-3de9-d74f-71005fbefb1d@gmail.com> <20fcad1c-a129-0d8a-d2fb-aac353c3089b@redhat.com> From: Jason Merrill In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Sep 2021 20:13:00 -0000 On 9/21/21 20:34, Martin Sebor wrote: > On 9/21/21 3:40 PM, Jason Merrill wrote: >> On 9/17/21 12:02, Martin Sebor wrote: >>> On 9/8/21 2:06 PM, Jason Merrill wrote: >>>> On 9/2/21 7:53 PM, Martin Sebor wrote: >>>>> @@ -4622,28 +4622,94 @@ warn_for_null_address (location_t location, >>>>> tree op, tsubst_flags_t complain) >>>>>     if (!warn_address >>>>>         || (complain & tf_warning) == 0 >>>>>         || c_inhibit_evaluation_warnings != 0 >>>>> -      || warning_suppressed_p (op, OPT_Waddress)) >>>>> +      || warning_suppressed_p (op, OPT_Waddress) >>>>> +      || processing_template_decl != 0) >>>> >>>> Completely suppressing this warning in templates seems like a >>>> regression;  I'd think we could recognize many relevant cases before >>>> instantiation.  You just can't assume that ADDR_EXPR has the default >>>> meaning if it has unknown type (i.e. because op0 is type-dependent). >>> >>> I added the suppression to keep g++.dg/warn/pr101219.C from failing >>> but in hindsight I should have questioned the reasoning behind >>> the "no warning emitted here (no instantiation)" comment in the test. >>> >>> I agree that it would be helpful to diagnose the type-independent >>> subset of the problem even in uninstantiated templates.  Current >>> trunk doesn't (it never has), but with my patch and the suppression >>> above removed it does.  I've updated the tests to expect it. >>> >>> Please see the attached revision. >>> >>> Martin >>> >>> PS There are still more opportunities to issue -Waddress in templates >>> that this patch doesn't handle, e.g.,: >>> >>>    template bool f (T *p) { return &p == 0; } >>> >>> Handling this will take more surgery. >>> >>> PPS It seems that most other warnings (and even some errors, like >>> -Wnarrowing) are suppressed in uninstantiated templates as well, >>> even for non-dependent expressions.  In the few test cases I looked >>> at Clang does better.  It sounds like you'd like to see improvements >>> in this direction not just for -Waddress but in general.  Just for >>> the avoidance of doubt, can you confirm that for future reference? >> >> Yes, in general it's better to diagnose sooner. >> >>> +  if (TREE_CODE (cop) == NON_LVALUE_EXPR) >>> +    /* Unwrap the expression for C++ 98.  */ >>> +    cop = TREE_OPERAND (cop, 0); >> >> What does this have to do with C++98? > > The code is needed to avoid failures in C++ 98 in the test below > where COP is a NON_LVALUE_EXPR which isn't handled below otherwise. > I didn't investigate why that happens (it works fine if f() is > an ordinary member function). > >   void f (bool); > >   void g () >   { >     struct A { virtual void vf (); }; > >     f (&A::vf);   // missing -Waddress in C++ 98 mode >   } > >> >>> +  if (TREE_CODE (cop) == PTRMEM_CST) >>> +    { >>> +      /* The address of a nonstatic data member is never null.  */ >>> +      warning_at (location, OPT_Waddress, >>> +          "the address %qE will never be NULL", >> >> Capitalizing NULL when talking about pointers-to-members seems a bit >> odd, but I guess it's fine. > > I agree.  My personal preference is for lowercase null (in all > languages) since that's the technical term for it.  I used NULL > here only to conform to the existing style.  I'm willing to > change all these warnings to either use null or to some form > that doesn't mention null (there are two in use, although if > I had my druthers I'd choose some other phrasing altogether). > Let me know if you would support such a change. I don't feel strongly about it. I agree that lowercase or another phrasing would be better, but probably better to avoid adding work for the translators with the churn. >> The C++ changes are OK. > > Jeff, should I take your previous "Generally OK" as an approval > for the rest of the patch as well?  (It has not changed in v2.) > I have just submitted a Glibc patch to suppress the new instances > there. > > Martin >