From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 47874 invoked by alias); 11 Dec 2018 16:59:34 -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 47559 invoked by uid 89); 11 Dec 2018 16:59:34 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=jasons, Jason's, jason's, Jasons X-HELO: mail-qt1-f195.google.com Received: from mail-qt1-f195.google.com (HELO mail-qt1-f195.google.com) (209.85.160.195) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 11 Dec 2018 16:59:32 +0000 Received: by mail-qt1-f195.google.com with SMTP id p17so17193438qtl.5 for ; Tue, 11 Dec 2018 08:59:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=7A7H00ddfz+VnahyX6yTTFn0GETJ0qoAOmktZFPoAIk=; b=V/XKWj6YUwMxr0VBIYfOUXLb59Hjsll7f+HPVXxbFIggEOohfk7kdOxryosCUbe/Ry 6uraVu3cww/Fb4+n3Kv6U/3nfimYVssIIRjR4zm38yz1Bg2AEcM/NWHgzxVbX4V51Dfs AIR6qRswclGs8LRwtIvTqfkv9FbhlKr4s7sOS/ryCCGrzLMS3XDLeGklRTZ7CNWsrH72 JYfoGpIv0vcsQJ1OJM+UiEV8XoKm4O+zbuXUhHwYb6FsOJuS2Hqg9QgkuyxqiUZ51+1W EpZ87iNULdRlr91ZlptlXydFfowhjmVNlrEn6q+R7Xt+csEZbMoXuf1ghX5joMvH9cAn hPaQ== Return-Path: Received: from localhost.localdomain (97-118-99-160.hlrn.qwest.net. [97.118.99.160]) by smtp.gmail.com with ESMTPSA id p3sm10243066qkp.48.2018.12.11.08.59.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 11 Dec 2018 08:59:29 -0800 (PST) Subject: Re: [PATCH] accept all C integer types in function parameters referenced by alloc_align (PR 88363) To: Jakub Jelinek , "Joseph S. Myers" , Marek Polacek , Jason Merrill , Nathan Sidwell Cc: Gcc Patch List References: <0f3f1395-adac-8b5f-82e4-e656bf1207fb@gmail.com> <20181211071726.GI12380@tucnak> From: Martin Sebor Message-ID: <78a7396e-8a64-4919-82d6-38959fda0e55@gmail.com> Date: Tue, 11 Dec 2018 16:59:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <20181211071726.GI12380@tucnak> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00723.txt.bz2 On 12/11/18 12:17 AM, Jakub Jelinek wrote: > On Mon, Dec 10, 2018 at 04:30:11PM -0700, Martin Sebor wrote: >> Some of my testing exposed a minor problem in GCC 9's validation >> of the type of function parameters referred to by attribute >> positional arguments. Whereas GCC 8 accepts all C integer types, >> including enumerated types, such as: >> >> enum AllocAlign { Align16 = 16, Align32 = 32 }; >> >> __attribute__ ((alloc_align (1))) void* >> alloc (size_t, enum AllocAlign) >> >> GCC 9 only accepts signed and unsigned integer types. This change >> (introduced by myself) was unintentional, and a fix for it is in >> the attached trivial patch. I plan to commit it without approval >> in the next day or so unless any concerns or suggestions come up. > > There is nothing obvious about this, so please don't commit it without > approval. See (*) below. > GCC 8 and older used to accept > even float or void * or struct arguments and just ignored them. > I think we need to discuss what types we want to allow without warnings and > what we should warn. Silently accepting invalid arguments like void* or structs and proceeding to ignore them is in my view a bug. Clang diagnoses both and that seems like the appropriate choice, so that's what I implemented in r266195 back in November. This patch doesn't change that; rather, it accepts more of the things that were accepted previously and that r266195 unintentionally rejected: bool, enums, and wide characters. I welcome feedback on this decision which is why I posted the patch here before checking it in. > As I wrote in the PR, I believe e.g. using alloc_align/alloc_size with > bool/_Bool is just a clear bug, you can store 0 or 1 in there, but e.g. > alignment 0 doesn't make sense. I'm fine with being more restrictive and rejecting bool. Clang accepts it but I agree that it's far more likely a bug in user code (and an oversight in Clang). > Enums are on the border line, I'll defer to C/C++ maintainers whether we > want to include that or not. From Jason's and Marek's responses it sounds like accepting enums here is appropriate. (Clang also accepts them.) I will go ahead and make the change for bool alone then. > > Jakub > [*] The change in the patch is obvious enough to me. All it does is accept more of the things that are accepted by GCC 8 (enums, bools, wchar_t, etc.) and that inadvertently started to be rejected as a result of my prior change. That the rules can be made more restrictive is something different. I recently brought up the question of the write w/o approval policy on the gcc list: https://gcc.gnu.org/ml/gcc/2018-11/msg00165.html looking for clarification. Except for Jeff's comment (which I'm afraid didn't really clarify things), didn't get any. You (the maintainers) have put it in place. If you don't intend for the rest of us to make use of it, or if it's not meant to be interpreted to give us the freedom to decide what is or isn't obvious, then change it. But it's disingenuous to claim that "We don't want to get overly restrictive about checkin policies" and then chastise people each time they say they might check something in on their own.