From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 61040 invoked by alias); 2 Jun 2017 21:28:03 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 60571 invoked by uid 89); 2 Jun 2017 21:28:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=offer X-HELO: mail-qt0-f172.google.com Received: from mail-qt0-f172.google.com (HELO mail-qt0-f172.google.com) (209.85.216.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 02 Jun 2017 21:28:01 +0000 Received: by mail-qt0-f172.google.com with SMTP id u12so17669043qth.0 for ; Fri, 02 Jun 2017 14:28:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=H/cxhexoqQyBe7+UsJWxCspu6H8iOUDn44rp9aWELU4=; b=Ep72E9hoUsLG0gq2M+jiLdWr3F0UkxSUHwHTFlMS9HqE3CzGWWd7uiqcgAienXUVnO wjcFwB7mLmaYaUNQsec7B+8A5QzK56/RkXRxmBLORT9rsLlu9fS/1EjwQK3X6U90tam/ PxwnOMvxsldXjLNbNBT32RLVcAFYiAvr34rGB7LtbENppSiaoHJIn7z2C0NOs0KDn/oq DANn+ieDlJRaHXnlDI3cAPkc0cdcBPJEwnbQwWnj1HXGH/+tiCELALnjvys/SEFdIHwO 2ULNFGJoSdFUooRxKRWTWCOPmG/SRGFGDwjtACEAT07RV4VwPqpCg6i4mIlVi5iJFjix YL8A== X-Gm-Message-State: AODbwcAXGMsF8pe2tqZvAZDFOhELv1a386r0gham1iFs5I1mwpHPzPko er9fZQ+dln8yvFxC X-Received: by 10.237.43.69 with SMTP id p63mr10828179qtd.200.1496438883708; Fri, 02 Jun 2017 14:28:03 -0700 (PDT) Received: from localhost.localdomain (71-218-22-76.hlrn.qwest.net. [71.218.22.76]) by smtp.gmail.com with ESMTPSA id r24sm15748716qtc.1.2017.06.02.14.28.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 02 Jun 2017 14:28:03 -0700 (PDT) Subject: Re: [PATCH] warn on mem calls modifying objects of non-trivial types (PR 80560) To: Jason Merrill References: <656ca1db-1082-b1ed-a911-ba7bf48f09c0@redhat.com> <3378da1e-189a-c997-8ec8-d3d4a9afbf98@gmail.com> <8d51dc8c-fe2d-e83d-7305-d272b9fb0247@redhat.com> <2743494a-9adf-7f57-6237-9932e3e53414@gmail.com> Cc: Pedro Alves , Gcc Patch List From: Martin Sebor Message-ID: Date: Fri, 02 Jun 2017 21:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00160.txt.bz2 On 05/31/2017 05:34 PM, Jason Merrill wrote: > On 05/27/2017 06:44 PM, Martin Sebor wrote: >> + /* True if the class is trivial and has a trivial non-deleted copy >> + assignment, copy ctor, and default ctor, respectively. The last >> + one isn't used to issue warnings but only to decide what suitable >> + alternatives to offer as replacements for the raw memory >> operation. */ >> + bool trivial = trivial_type_p (desttype); > > This comment seems out of date; there's only one variable here now. > >> + /* True if the class is has a non-deleted trivial assignment. Set > > s/is// > >> + /* True if the class has a (possibly deleted) trivial copy ctor. */ >> + bool trivcopy = trivially_copyable_p (desttype); > > "True if the class is trivially copyable." > >> + if (delassign) >> + warnfmt = G_("%qD writing to an object of type %#qT with " >> + "deleted copy assignment"); >> + else if (!trivassign) >> + warnfmt = G_("%qD writing to an object of type %#qT with " >> + "no trivial copy assignment"); >> + else if (!trivial) >> + warnfmt = G_("%qD writing to an object of non-trivial " >> + "type %#qT; use assignment instead"); > > I'd still like the !trivial test to come first in all the memset cases, > !trivcopy in the copy cases. The tests are in the order they're in to provide as much useful detail in the diagnostics as necessary to understand the problem make the suggestion meaningful. To what end you want to change it? AFAICS, all it will accomplish is shuffle the tests around because starting with !trivial means I'll still need to test for delassign and delcopy before issuing the warning so that I can include the right suggestion (and avoid suggesting to use assignment when it's not available). >> +static bool >> +has_trivial_special_function (tree ctype, special_function_kind sfk, >> + bool *deleted_p) > > This seems redundant with type_has_trivial_fn. If that function is > giving the wrong answer for a class where all of the SFK are deleted, > let's fix that, under check_bases_and_members, rather than introduce a > new function. I don't want to call synthesized_method_walk every time > we want to check whether a function is trivial. A deleted special function can be trivial. type_has_trivial_fn() only determines whether a function is trivial, not whether it's deleted. I chose not to use the function because when a special function is deleted I don't want to have the warning suggest it as an alternative to the raw memory function. Maybe I should use a different approach and instead of trying to see if a function is deleted use trivially_xible to see if it's usable. That will mean changing the diagnostics from "with a deleted special function" to "without trivial special function" but it will avoid calling synthesized_method_walk while still avoiding giving bogus suggestions. Is this approach acceptable? Martin