Re: [PATCH] c: Implement C23 nullptr (N3042)

[Joseph Myers <joseph@codesourcery.com>]

On Sat, 13 Aug 2022, Marek Polacek via Gcc-patches wrote:

> This patch also defines nullptr_t in <stddef.h>.  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