From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 2D0633858D32 for ; Mon, 15 Aug 2022 17:48:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2D0633858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.93,238,1654588800"; d="scan'208";a="84057430" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa1.mentor.iphmx.com with ESMTP; 15 Aug 2022 09:48:38 -0800 IronPort-SDR: uS39+BtQ1OmFYkCACftZy8ukNiQhejqHG0n9aGRX6JM1Upi9SS9xgpAOzcrYXeWf3mbf3FGQ1I zYhu7yT00eifUrpKOSNjJspwVAkWaGbVJ8Vec8W4GJxUrlbmfpYHaBDUyFkD5W5NbftD0O5A3j lfsCdWBnj9iRzr/rAjtwOrnGiasyFF8IvplRf1ekT3wkR8Of2K4siqiAvIiCqSvU/lAOosaRbD q5HsCdNZU956revRFcrmI20ejufawvWMnPNpxkEqrqkuXVqd/msdnwuwUdD9EyXIQwmGRUbnAO X+I= Date: Mon, 15 Aug 2022 17:48:34 +0000 From: Joseph Myers X-X-Sender: jsm28@digraph.polyomino.org.uk To: Marek Polacek CC: GCC Patches , Jason Merrill Subject: Re: [PATCH] c: Implement C23 nullptr (N3042) In-Reply-To: <20220813213504.568937-1-polacek@redhat.com> Message-ID: References: <20220813213504.568937-1-polacek@redhat.com> User-Agent: Alpine 2.22 (DEB 394 2020-01-19) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: SVR-IES-MBX-07.mgc.mentorg.com (139.181.222.7) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-3117.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Mon, 15 Aug 2022 17:48:42 -0000 On Sat, 13 Aug 2022, Marek Polacek via Gcc-patches wrote: > This patch also defines nullptr_t in . I'm uncertain about > the __STDC_VERSION__ version I should be checking. Also, I'm not We're using defined (__STDC_VERSION__) && __STDC_VERSION__ > 201710L until the final version for C23 is settled. > defining __STDC_VERSION_STDDEF_H__ yet, because I don't know what value > it should be defined to. Do we know yet? No, and Jens's comments on the editorial review before CD ballot include that lots of headers don't yet have such a macro definition but should have one, as well as needing consistency for the numbers. We won't know the final values for these macros until much later, because the timescale depends on whether ISO decides to delay things at any point by coming up with a long list of editorial issues required to follow the JTC1 Directives as they did for C17 (objections to particular words appearing in non-normative text, etc.). > - { "nullptr", RID_NULLPTR, D_CXXONLY | D_CXX11 | D_CXXWARN }, > + { "nullptr", RID_NULLPTR, D_CXX11 | D_CXXWARN }, You need to use D_C2X (which doesn't yet exist). In pre-C23 modes, nullptr needs to be a normal identifier that can be used in all contexts where identifiers can be used, not a keyword at all (and then you need a c11-nullptr*.c test to verify that use of it as an identifier works as expected). > diff --git a/gcc/c/c-convert.cc b/gcc/c/c-convert.cc > index 18083d59618..013fe6b2a53 100644 > --- a/gcc/c/c-convert.cc > +++ b/gcc/c/c-convert.cc > @@ -133,6 +133,14 @@ c_convert (tree type, tree expr, bool init_const) > (loc, type, c_objc_common_truthvalue_conversion (input_location, expr)); > > case POINTER_TYPE: > + /* The type nullptr_t may be converted to a pointer type. The result is > + a null pointer value. */ > + if (NULLPTR_TYPE_P (TREE_TYPE (e))) > + { > + ret = build_int_cst (type, 0); > + goto maybe_fold; > + } That looks like it would lose side-effects. You need to preserve side-effects in an expression of nullptr_t type being converted to a pointer type, and need an execution testcase that verifies such side-effects are preserved. Also, you need to make sure that (void *)nullptr is not treated as a null pointer constant, only a null pointer; build_int_cst (type, 0) would produce a null pointer constant when type is void *. (void *)nullptr should be handled similarly to (void *)(void *)0, which isn't a null pointer constant either. > @@ -133,6 +133,13 @@ null_pointer_constant_p (const_tree expr) > /* This should really operate on c_expr structures, but they aren't > yet available everywhere required. */ > tree type = TREE_TYPE (expr); > + > + /* An integer constant expression with the value 0, such an expression > + cast to type void*, or the predefined constant nullptr, are a null > + pointer constant. */ > + if (NULLPTR_TYPE_P (type)) > + return true; That looks wrong. You need to distinguish null pointer constants of type nullptr_t (nullptr, possibly enclosed in parentheses, possibly the selected alternative from _Generic) from all other expressions of type nullptr_t (including (nullptr_t)nullptr, which isn't a null pointer constant any more than (void *)(void *)0). Then, for each context where it matters whether a nullptr_t value is a null pointer constant, there need to be testcases that the two cases are properly distinguished. This includes at least equality comparisons with a pointer that is not a null pointer constant (seem only to be allowed with nullptr, not with other nullptr_t expressions). (I think for conditional expressions, conditionals between nullptr_t and an integer null pointer constant are always invalid, whether or not the nullptr_t is a null pointer constant, while conditionals between nullptr_t and a pointer are always valid.) > +/* The type nullptr_t shall not be converted to any type other than bool or > + a pointer type. No type other than nullptr_t shall be converted to nullptr_t. */ That's other than *void*, bool or a pointer type. (That's a correct fix to the N3042 wording in N3047. There are other problems in the integration of nullptr in N3047 that are only fixed in my subsequent fixes as part of the editorial review - and many issues with integration of other papers that haven't yet been fixed, I currently have 25 open merge requests resulting from editorial review.) And of course conversions from nullptr_t to void should be tested. > diff --git a/gcc/testsuite/gcc.dg/c2x-nullptr-4.c b/gcc/testsuite/gcc.dg/c2x-nullptr-4.c > new file mode 100644 > index 00000000000..5b15e75d159 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/c2x-nullptr-4.c > @@ -0,0 +1,10 @@ > +/* Test that we warn about `nullptr' pre-C2X. */ > +/* { dg-do compile } */ > +/* { dg-options "-std=c17 -pedantic-errors" } */ This test is wrong - it's a normal identifier pre-C2x - but tests for previous standard versions shouldn't be called c2x-* in any case. -- Joseph S. Myers joseph@codesourcery.com