public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Improve diagnostics for constexpr cast from void*
@ 2023-10-09 10:03 Nathaniel Shead
  2023-10-09 20:10 ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Nathaniel Shead @ 2023-10-09 10:03 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill

Bootstrapped and regtested on x86_64-pc-linux-gnu with 
GXX_TESTSUITE_STDS=98,11,14,17,20,23,26,impcx.

-- >8 --

This patch improves the errors given when casting from void* in C++26 to
include the expected type if the type of the pointed-to object was
not similar to the casted-to type. 

It also ensures (for all standard modes) that void* casts are checked
even for DECL_ARTIFICIAL declarations, such as lifetime-extended
temporaries, and is only ignored for cases where we know it's OK (heap
identifiers and source_location::current). This provides more accurate
diagnostics when using the pointer and ensures that some other casts
from void* are now correctly rejected.

gcc/cp/ChangeLog:

	* constexpr.cc (is_std_source_location_current): New.
	(cxx_eval_constant_expression): Only ignore cast from void* for
	specific cases and improve other diagnostics.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/constexpr-cast4.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/constexpr.cc                          | 83 +++++++++++++++++---
 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C |  7 ++
 2 files changed, 78 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0f948db7c2d..f38d541a662 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
 	  && is_std_allocator_allocate (call->fundef->decl));
 }
 
+/* Return true if FNDECL is std::source_location::current.  */
+
+static inline bool
+is_std_source_location_current (tree fndecl)
+{
+  if (!decl_in_std_namespace_p (fndecl))
+    return false;
+
+  tree name = DECL_NAME (fndecl);
+  if (name == NULL_TREE || !id_equal (name, "current"))
+    return false;
+
+  tree ctx = DECL_CONTEXT (fndecl);
+  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
+    return false;
+
+  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
+  return name && id_equal (name, "source_location");
+}
+
+/* Overload for the above taking constexpr_call*.  */
+
+static inline bool
+is_std_source_location_current (const constexpr_call *call)
+{
+  return (call
+	  && call->fundef
+	  && is_std_source_location_current (call->fundef->decl));
+}
+
 /* Return true if FNDECL is __dynamic_cast.  */
 
 static inline bool
@@ -7850,33 +7880,62 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	if (TYPE_PTROB_P (type)
 	    && TYPE_PTR_P (TREE_TYPE (op))
 	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
-	    /* Inside a call to std::construct_at or to
-	       std::allocator<T>::{,de}allocate, we permit casting from void*
+	    /* Inside a call to std::construct_at,
+	       std::allocator<T>::{,de}allocate, or
+	       std::source_location::current, we permit casting from void*
 	       because that is compiler-generated code.  */
 	    && !is_std_construct_at (ctx->call)
-	    && !is_std_allocator_allocate (ctx->call))
+	    && !is_std_allocator_allocate (ctx->call)
+	    && !is_std_source_location_current (ctx->call))
 	  {
 	    /* Likewise, don't error when casting from void* when OP is
 	       &heap uninit and similar.  */
 	    tree sop = tree_strip_nop_conversions (op);
-	    if (TREE_CODE (sop) == ADDR_EXPR
-		&& VAR_P (TREE_OPERAND (sop, 0))
-		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
+	    tree decl = NULL_TREE;
+	    if (TREE_CODE (sop) == ADDR_EXPR)
+	      decl = TREE_OPERAND (sop, 0);
+	    if (decl
+		&& VAR_P (decl)
+		&& DECL_ARTIFICIAL (decl)
+		&& (DECL_NAME (decl) == heap_identifier
+		    || DECL_NAME (decl) == heap_uninit_identifier
+		    || DECL_NAME (decl) == heap_vec_identifier
+		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
 	      /* OK */;
 	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
 	       cv void" to a pointer-to-object type T unless P points to an
 	       object whose type is similar to T.  */
-	    else if (cxx_dialect > cxx23
-		     && (sop = cxx_fold_indirect_ref (ctx, loc,
-						      TREE_TYPE (type), sop)))
+	    else if (cxx_dialect > cxx23)
 	      {
-		r = build1 (ADDR_EXPR, type, sop);
-		break;
+		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
+		if (r)
+		  {
+		    r = build1 (ADDR_EXPR, type, r);
+		    break;
+		  }
+		if (!ctx->quiet)
+		  {
+		    if (TREE_CODE (sop) == ADDR_EXPR)
+		      {
+			error_at (loc, "cast from %qT is not allowed because "
+				  "pointed-to type %qT is not similar to %qT",
+				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
+				  TREE_TYPE (type));
+			tree obj = build_fold_indirect_ref (sop);
+			inform (DECL_SOURCE_LOCATION (obj),
+				"pointed-to object declared here");
+		      }
+		    else
+		      error_at (loc, "cast from %qT is not allowed",
+				TREE_TYPE (op));
+		  }
+		*non_constant_p = true;
+		return t;
 	      }
 	    else
 	      {
 		if (!ctx->quiet)
-		  error_at (loc, "cast from %qT is not allowed",
+		  error_at (loc, "cast from %qT is not allowed before C++26",
 			    TREE_TYPE (op));
 		*non_constant_p = true;
 		return t;
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
new file mode 100644
index 00000000000..0e7fd87add8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
@@ -0,0 +1,7 @@
+// { dg-do compile { target c++11 } }
+
+constexpr int&& r = 1 + 2;  // { dg-message "pointed-to object declared here" "" { target c++26 } }
+constexpr void *vpr = &r;
+constexpr int* pi = static_cast<int *>(vpr);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+constexpr float* pf = static_cast<float *>(vpr);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+// { dg-error "cast from .void\\*. is not allowed because pointed-to type .int. is not similar to .float." "" { target c++26 } .-1 }
-- 
2.42.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] c++: Improve diagnostics for constexpr cast from void*
  2023-10-09 10:03 [PATCH] c++: Improve diagnostics for constexpr cast from void* Nathaniel Shead
@ 2023-10-09 20:10 ` Jason Merrill
  2023-10-10 23:57   ` [PATCH v2] " Nathaniel Shead
  0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2023-10-09 20:10 UTC (permalink / raw)
  To: Nathaniel Shead, gcc-patches

On 10/9/23 06:03, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu with
> GXX_TESTSUITE_STDS=98,11,14,17,20,23,26,impcx.
> 
> -- >8 --
> 
> This patch improves the errors given when casting from void* in C++26 to
> include the expected type if the type of the pointed-to object was
> not similar to the casted-to type.
> 
> It also ensures (for all standard modes) that void* casts are checked
> even for DECL_ARTIFICIAL declarations, such as lifetime-extended
> temporaries, and is only ignored for cases where we know it's OK (heap
> identifiers and source_location::current). This provides more accurate
> diagnostics when using the pointer and ensures that some other casts
> from void* are now correctly rejected.
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (is_std_source_location_current): New.
> 	(cxx_eval_constant_expression): Only ignore cast from void* for
> 	specific cases and improve other diagnostics.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/constexpr-cast4.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/constexpr.cc                          | 83 +++++++++++++++++---
>   gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C |  7 ++
>   2 files changed, 78 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 0f948db7c2d..f38d541a662 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
>   	  && is_std_allocator_allocate (call->fundef->decl));
>   }
>   
> +/* Return true if FNDECL is std::source_location::current.  */
> +
> +static inline bool
> +is_std_source_location_current (tree fndecl)
> +{
> +  if (!decl_in_std_namespace_p (fndecl))
> +    return false;
> +
> +  tree name = DECL_NAME (fndecl);
> +  if (name == NULL_TREE || !id_equal (name, "current"))
> +    return false;
> +
> +  tree ctx = DECL_CONTEXT (fndecl);
> +  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
> +    return false;
> +
> +  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
> +  return name && id_equal (name, "source_location");
> +}
> +
> +/* Overload for the above taking constexpr_call*.  */
> +
> +static inline bool
> +is_std_source_location_current (const constexpr_call *call)
> +{
> +  return (call
> +	  && call->fundef
> +	  && is_std_source_location_current (call->fundef->decl));
> +}
> +
>   /* Return true if FNDECL is __dynamic_cast.  */
>   
>   static inline bool
> @@ -7850,33 +7880,62 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>   	if (TYPE_PTROB_P (type)
>   	    && TYPE_PTR_P (TREE_TYPE (op))
>   	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
> -	    /* Inside a call to std::construct_at or to
> -	       std::allocator<T>::{,de}allocate, we permit casting from void*
> +	    /* Inside a call to std::construct_at,
> +	       std::allocator<T>::{,de}allocate, or
> +	       std::source_location::current, we permit casting from void*
>   	       because that is compiler-generated code.  */
>   	    && !is_std_construct_at (ctx->call)
> -	    && !is_std_allocator_allocate (ctx->call))
> +	    && !is_std_allocator_allocate (ctx->call)
> +	    && !is_std_source_location_current (ctx->call))
>   	  {
>   	    /* Likewise, don't error when casting from void* when OP is
>   	       &heap uninit and similar.  */
>   	    tree sop = tree_strip_nop_conversions (op);
> -	    if (TREE_CODE (sop) == ADDR_EXPR
> -		&& VAR_P (TREE_OPERAND (sop, 0))
> -		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
> +	    tree decl = NULL_TREE;
> +	    if (TREE_CODE (sop) == ADDR_EXPR)
> +	      decl = TREE_OPERAND (sop, 0);
> +	    if (decl
> +		&& VAR_P (decl)
> +		&& DECL_ARTIFICIAL (decl)
> +		&& (DECL_NAME (decl) == heap_identifier
> +		    || DECL_NAME (decl) == heap_uninit_identifier
> +		    || DECL_NAME (decl) == heap_vec_identifier
> +		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
>   	      /* OK */;
>   	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
>   	       cv void" to a pointer-to-object type T unless P points to an
>   	       object whose type is similar to T.  */
> -	    else if (cxx_dialect > cxx23
> -		     && (sop = cxx_fold_indirect_ref (ctx, loc,
> -						      TREE_TYPE (type), sop)))
> +	    else if (cxx_dialect > cxx23)
>   	      {
> -		r = build1 (ADDR_EXPR, type, sop);
> -		break;
> +		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
> +		if (r)
> +		  {
> +		    r = build1 (ADDR_EXPR, type, r);
> +		    break;
> +		  }
> +		if (!ctx->quiet)
> +		  {
> +		    if (TREE_CODE (sop) == ADDR_EXPR)
> +		      {
> +			error_at (loc, "cast from %qT is not allowed because "
> +				  "pointed-to type %qT is not similar to %qT",
> +				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
> +				  TREE_TYPE (type));
> +			tree obj = build_fold_indirect_ref (sop);
> +			inform (DECL_SOURCE_LOCATION (obj),
> +				"pointed-to object declared here");
> +		      }
> +		    else
> +		      error_at (loc, "cast from %qT is not allowed",

Can this be more helpful as well, i.e. say because op is not the address 
of an object of similar type?

Can we only get here if op is null, since we would have returned already 
for non-constant op?

Jason


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] c++: Improve diagnostics for constexpr cast from void*
  2023-10-09 20:10 ` Jason Merrill
@ 2023-10-10 23:57   ` Nathaniel Shead
  2023-10-11 15:41     ` Marek Polacek
  0 siblings, 1 reply; 5+ messages in thread
From: Nathaniel Shead @ 2023-10-10 23:57 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Mon, Oct 09, 2023 at 04:10:20PM -0400, Jason Merrill wrote:
> On 10/9/23 06:03, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu with
> > GXX_TESTSUITE_STDS=98,11,14,17,20,23,26,impcx.
> > 
> > -- >8 --
> > 
> > This patch improves the errors given when casting from void* in C++26 to
> > include the expected type if the type of the pointed-to object was
> > not similar to the casted-to type.
> > 
> > It also ensures (for all standard modes) that void* casts are checked
> > even for DECL_ARTIFICIAL declarations, such as lifetime-extended
> > temporaries, and is only ignored for cases where we know it's OK (heap
> > identifiers and source_location::current). This provides more accurate
> > diagnostics when using the pointer and ensures that some other casts
> > from void* are now correctly rejected.
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* constexpr.cc (is_std_source_location_current): New.
> > 	(cxx_eval_constant_expression): Only ignore cast from void* for
> > 	specific cases and improve other diagnostics.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp0x/constexpr-cast4.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> >   gcc/cp/constexpr.cc                          | 83 +++++++++++++++++---
> >   gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C |  7 ++
> >   2 files changed, 78 insertions(+), 12 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
> > 
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index 0f948db7c2d..f38d541a662 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
> >   	  && is_std_allocator_allocate (call->fundef->decl));
> >   }
> > +/* Return true if FNDECL is std::source_location::current.  */
> > +
> > +static inline bool
> > +is_std_source_location_current (tree fndecl)
> > +{
> > +  if (!decl_in_std_namespace_p (fndecl))
> > +    return false;
> > +
> > +  tree name = DECL_NAME (fndecl);
> > +  if (name == NULL_TREE || !id_equal (name, "current"))
> > +    return false;
> > +
> > +  tree ctx = DECL_CONTEXT (fndecl);
> > +  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
> > +    return false;
> > +
> > +  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
> > +  return name && id_equal (name, "source_location");
> > +}
> > +
> > +/* Overload for the above taking constexpr_call*.  */
> > +
> > +static inline bool
> > +is_std_source_location_current (const constexpr_call *call)
> > +{
> > +  return (call
> > +	  && call->fundef
> > +	  && is_std_source_location_current (call->fundef->decl));
> > +}
> > +
> >   /* Return true if FNDECL is __dynamic_cast.  */
> >   static inline bool
> > @@ -7850,33 +7880,62 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> >   	if (TYPE_PTROB_P (type)
> >   	    && TYPE_PTR_P (TREE_TYPE (op))
> >   	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
> > -	    /* Inside a call to std::construct_at or to
> > -	       std::allocator<T>::{,de}allocate, we permit casting from void*
> > +	    /* Inside a call to std::construct_at,
> > +	       std::allocator<T>::{,de}allocate, or
> > +	       std::source_location::current, we permit casting from void*
> >   	       because that is compiler-generated code.  */
> >   	    && !is_std_construct_at (ctx->call)
> > -	    && !is_std_allocator_allocate (ctx->call))
> > +	    && !is_std_allocator_allocate (ctx->call)
> > +	    && !is_std_source_location_current (ctx->call))
> >   	  {
> >   	    /* Likewise, don't error when casting from void* when OP is
> >   	       &heap uninit and similar.  */
> >   	    tree sop = tree_strip_nop_conversions (op);
> > -	    if (TREE_CODE (sop) == ADDR_EXPR
> > -		&& VAR_P (TREE_OPERAND (sop, 0))
> > -		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
> > +	    tree decl = NULL_TREE;
> > +	    if (TREE_CODE (sop) == ADDR_EXPR)
> > +	      decl = TREE_OPERAND (sop, 0);
> > +	    if (decl
> > +		&& VAR_P (decl)
> > +		&& DECL_ARTIFICIAL (decl)
> > +		&& (DECL_NAME (decl) == heap_identifier
> > +		    || DECL_NAME (decl) == heap_uninit_identifier
> > +		    || DECL_NAME (decl) == heap_vec_identifier
> > +		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
> >   	      /* OK */;
> >   	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
> >   	       cv void" to a pointer-to-object type T unless P points to an
> >   	       object whose type is similar to T.  */
> > -	    else if (cxx_dialect > cxx23
> > -		     && (sop = cxx_fold_indirect_ref (ctx, loc,
> > -						      TREE_TYPE (type), sop)))
> > +	    else if (cxx_dialect > cxx23)
> >   	      {
> > -		r = build1 (ADDR_EXPR, type, sop);
> > -		break;
> > +		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
> > +		if (r)
> > +		  {
> > +		    r = build1 (ADDR_EXPR, type, r);
> > +		    break;
> > +		  }
> > +		if (!ctx->quiet)
> > +		  {
> > +		    if (TREE_CODE (sop) == ADDR_EXPR)
> > +		      {
> > +			error_at (loc, "cast from %qT is not allowed because "
> > +				  "pointed-to type %qT is not similar to %qT",
> > +				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
> > +				  TREE_TYPE (type));
> > +			tree obj = build_fold_indirect_ref (sop);
> > +			inform (DECL_SOURCE_LOCATION (obj),
> > +				"pointed-to object declared here");
> > +		      }
> > +		    else
> > +		      error_at (loc, "cast from %qT is not allowed",
> 
> Can this be more helpful as well, i.e. say because op is not the address of
> an object of similar type?
> 
> Can we only get here if op is null, since we would have returned already for
> non-constant op?
> 
> Jason
> 

Hm, yes; I'd kept the error as it was because I wasn't sure what other
kinds of trees might end up here, but I've done a fair amount of testing
and I've only been able to reach here with null pointers: everything
else gets caught earlier. I've updated the error message appropriately
and documented this assumption with an assertion.

Bootstrapped + regtested on x86_64-pc-linux-gnu as above.

-- >8 --

This patch improves the errors given when casting from void* in C++26 to
include the expected type if the types of the pointed-to objects were
not similar. It also ensures (for all standard modes) that void* casts
are checked even for DECL_ARTIFICIAL declarations, such as
lifetime-extended temporaries, and is only ignored for cases where we
know it's OK (e.g. source_location::current) or have no other choice
(heap-allocated data).

gcc/cp/ChangeLog:

	* constexpr.cc (is_std_source_location_current): New.
	(cxx_eval_constant_expression): Only ignore cast from void* for
	specific cases and improve other diagnostics.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/constexpr-cast4.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/constexpr.cc                          | 87 +++++++++++++++++---
 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C | 11 +++
 2 files changed, 86 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 0f948db7c2d..d060e374cb3 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
 	  && is_std_allocator_allocate (call->fundef->decl));
 }
 
+/* Return true if FNDECL is std::source_location::current.  */
+
+static inline bool
+is_std_source_location_current (tree fndecl)
+{
+  if (!decl_in_std_namespace_p (fndecl))
+    return false;
+
+  tree name = DECL_NAME (fndecl);
+  if (name == NULL_TREE || !id_equal (name, "current"))
+    return false;
+
+  tree ctx = DECL_CONTEXT (fndecl);
+  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
+    return false;
+
+  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
+  return name && id_equal (name, "source_location");
+}
+
+/* Overload for the above taking constexpr_call*.  */
+
+static inline bool
+is_std_source_location_current (const constexpr_call *call)
+{
+  return (call
+	  && call->fundef
+	  && is_std_source_location_current (call->fundef->decl));
+}
+
 /* Return true if FNDECL is __dynamic_cast.  */
 
 static inline bool
@@ -7850,33 +7880,66 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	if (TYPE_PTROB_P (type)
 	    && TYPE_PTR_P (TREE_TYPE (op))
 	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
-	    /* Inside a call to std::construct_at or to
-	       std::allocator<T>::{,de}allocate, we permit casting from void*
+	    /* Inside a call to std::construct_at,
+	       std::allocator<T>::{,de}allocate, or
+	       std::source_location::current, we permit casting from void*
 	       because that is compiler-generated code.  */
 	    && !is_std_construct_at (ctx->call)
-	    && !is_std_allocator_allocate (ctx->call))
+	    && !is_std_allocator_allocate (ctx->call)
+	    && !is_std_source_location_current (ctx->call))
 	  {
 	    /* Likewise, don't error when casting from void* when OP is
 	       &heap uninit and similar.  */
 	    tree sop = tree_strip_nop_conversions (op);
-	    if (TREE_CODE (sop) == ADDR_EXPR
-		&& VAR_P (TREE_OPERAND (sop, 0))
-		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
+	    tree decl = NULL_TREE;
+	    if (TREE_CODE (sop) == ADDR_EXPR)
+	      decl = TREE_OPERAND (sop, 0);
+	    if (decl
+		&& VAR_P (decl)
+		&& DECL_ARTIFICIAL (decl)
+		&& (DECL_NAME (decl) == heap_identifier
+		    || DECL_NAME (decl) == heap_uninit_identifier
+		    || DECL_NAME (decl) == heap_vec_identifier
+		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
 	      /* OK */;
 	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
 	       cv void" to a pointer-to-object type T unless P points to an
 	       object whose type is similar to T.  */
-	    else if (cxx_dialect > cxx23
-		     && (sop = cxx_fold_indirect_ref (ctx, loc,
-						      TREE_TYPE (type), sop)))
+	    else if (cxx_dialect > cxx23)
 	      {
-		r = build1 (ADDR_EXPR, type, sop);
-		break;
+		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
+		if (r)
+		  {
+		    r = build1 (ADDR_EXPR, type, r);
+		    break;
+		  }
+		if (!ctx->quiet)
+		  {
+		    if (TREE_CODE (sop) == ADDR_EXPR)
+		      {
+			error_at (loc, "cast from %qT is not allowed because "
+				  "pointed-to type %qT is not similar to %qT",
+				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
+				  TREE_TYPE (type));
+			tree obj = build_fold_indirect_ref (sop);
+			inform (DECL_SOURCE_LOCATION (obj),
+				"pointed-to object declared here");
+		      }
+		    else
+		      {
+			gcc_assert (integer_zerop (sop));
+			error_at (loc, "cast from %qT is not allowed because "
+				  "%qE does not point to an object",
+				  TREE_TYPE (op), oldop);
+		      }
+		  }
+		*non_constant_p = true;
+		return t;
 	      }
 	    else
 	      {
 		if (!ctx->quiet)
-		  error_at (loc, "cast from %qT is not allowed",
+		  error_at (loc, "cast from %qT is not allowed before C++26",
 			    TREE_TYPE (op));
 		*non_constant_p = true;
 		return t;
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
new file mode 100644
index 00000000000..cfeaa1ac3bf
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
@@ -0,0 +1,11 @@
+// { dg-do compile { target c++11 } }
+
+constexpr int&& r = 1 + 2;  // { dg-message "pointed-to object declared here" "" { target c++26 } }
+constexpr void* vpr = &r;
+constexpr int* pi = static_cast<int*>(vpr);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+constexpr float* pf = static_cast<float*>(vpr);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+// { dg-error "cast from .void\\*. is not allowed because pointed-to type .int. is not similar to .float." "" { target c++26 } .-1 }
+
+constexpr void* vnp = nullptr;
+constexpr int* pi2 = static_cast<int*>(vnp);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+// { dg-error "cast from .void\\*. is not allowed because .vnp. does not point to an object" "" { target c++26 } .-1 }
-- 
2.42.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] c++: Improve diagnostics for constexpr cast from void*
  2023-10-10 23:57   ` [PATCH v2] " Nathaniel Shead
@ 2023-10-11 15:41     ` Marek Polacek
  2023-10-20  3:25       ` Jason Merrill
  0 siblings, 1 reply; 5+ messages in thread
From: Marek Polacek @ 2023-10-11 15:41 UTC (permalink / raw)
  To: Nathaniel Shead; +Cc: Jason Merrill, gcc-patches

On Wed, Oct 11, 2023 at 10:57:06AM +1100, Nathaniel Shead wrote:
> On Mon, Oct 09, 2023 at 04:10:20PM -0400, Jason Merrill wrote:
> > On 10/9/23 06:03, Nathaniel Shead wrote:
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu with
> > > GXX_TESTSUITE_STDS=98,11,14,17,20,23,26,impcx.
> > > 
> > > -- >8 --
> > > 
> > > This patch improves the errors given when casting from void* in C++26 to
> > > include the expected type if the type of the pointed-to object was
> > > not similar to the casted-to type.
> > > 
> > > It also ensures (for all standard modes) that void* casts are checked
> > > even for DECL_ARTIFICIAL declarations, such as lifetime-extended
> > > temporaries, and is only ignored for cases where we know it's OK (heap
> > > identifiers and source_location::current). This provides more accurate
> > > diagnostics when using the pointer and ensures that some other casts
> > > from void* are now correctly rejected.
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	* constexpr.cc (is_std_source_location_current): New.
> > > 	(cxx_eval_constant_expression): Only ignore cast from void* for
> > > 	specific cases and improve other diagnostics.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* g++.dg/cpp0x/constexpr-cast4.C: New test.
> > > 
> > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > ---
> > >   gcc/cp/constexpr.cc                          | 83 +++++++++++++++++---
> > >   gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C |  7 ++
> > >   2 files changed, 78 insertions(+), 12 deletions(-)
> > >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
> > > 
> > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > > index 0f948db7c2d..f38d541a662 100644
> > > --- a/gcc/cp/constexpr.cc
> > > +++ b/gcc/cp/constexpr.cc
> > > @@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
> > >   	  && is_std_allocator_allocate (call->fundef->decl));
> > >   }
> > > +/* Return true if FNDECL is std::source_location::current.  */
> > > +
> > > +static inline bool
> > > +is_std_source_location_current (tree fndecl)
> > > +{
> > > +  if (!decl_in_std_namespace_p (fndecl))
> > > +    return false;
> > > +
> > > +  tree name = DECL_NAME (fndecl);
> > > +  if (name == NULL_TREE || !id_equal (name, "current"))
> > > +    return false;
> > > +
> > > +  tree ctx = DECL_CONTEXT (fndecl);
> > > +  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
> > > +    return false;
> > > +
> > > +  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
> > > +  return name && id_equal (name, "source_location");
> > > +}
> > > +
> > > +/* Overload for the above taking constexpr_call*.  */
> > > +
> > > +static inline bool
> > > +is_std_source_location_current (const constexpr_call *call)
> > > +{
> > > +  return (call
> > > +	  && call->fundef
> > > +	  && is_std_source_location_current (call->fundef->decl));
> > > +}
> > > +
> > >   /* Return true if FNDECL is __dynamic_cast.  */
> > >   static inline bool
> > > @@ -7850,33 +7880,62 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
> > >   	if (TYPE_PTROB_P (type)
> > >   	    && TYPE_PTR_P (TREE_TYPE (op))
> > >   	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
> > > -	    /* Inside a call to std::construct_at or to
> > > -	       std::allocator<T>::{,de}allocate, we permit casting from void*
> > > +	    /* Inside a call to std::construct_at,
> > > +	       std::allocator<T>::{,de}allocate, or
> > > +	       std::source_location::current, we permit casting from void*
> > >   	       because that is compiler-generated code.  */
> > >   	    && !is_std_construct_at (ctx->call)
> > > -	    && !is_std_allocator_allocate (ctx->call))
> > > +	    && !is_std_allocator_allocate (ctx->call)
> > > +	    && !is_std_source_location_current (ctx->call))
> > >   	  {
> > >   	    /* Likewise, don't error when casting from void* when OP is
> > >   	       &heap uninit and similar.  */
> > >   	    tree sop = tree_strip_nop_conversions (op);
> > > -	    if (TREE_CODE (sop) == ADDR_EXPR
> > > -		&& VAR_P (TREE_OPERAND (sop, 0))
> > > -		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
> > > +	    tree decl = NULL_TREE;
> > > +	    if (TREE_CODE (sop) == ADDR_EXPR)
> > > +	      decl = TREE_OPERAND (sop, 0);
> > > +	    if (decl
> > > +		&& VAR_P (decl)
> > > +		&& DECL_ARTIFICIAL (decl)
> > > +		&& (DECL_NAME (decl) == heap_identifier
> > > +		    || DECL_NAME (decl) == heap_uninit_identifier
> > > +		    || DECL_NAME (decl) == heap_vec_identifier
> > > +		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
> > >   	      /* OK */;
> > >   	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
> > >   	       cv void" to a pointer-to-object type T unless P points to an
> > >   	       object whose type is similar to T.  */
> > > -	    else if (cxx_dialect > cxx23
> > > -		     && (sop = cxx_fold_indirect_ref (ctx, loc,
> > > -						      TREE_TYPE (type), sop)))
> > > +	    else if (cxx_dialect > cxx23)
> > >   	      {
> > > -		r = build1 (ADDR_EXPR, type, sop);
> > > -		break;
> > > +		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
> > > +		if (r)
> > > +		  {
> > > +		    r = build1 (ADDR_EXPR, type, r);
> > > +		    break;
> > > +		  }
> > > +		if (!ctx->quiet)
> > > +		  {
> > > +		    if (TREE_CODE (sop) == ADDR_EXPR)
> > > +		      {
> > > +			error_at (loc, "cast from %qT is not allowed because "
> > > +				  "pointed-to type %qT is not similar to %qT",
> > > +				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
> > > +				  TREE_TYPE (type));
> > > +			tree obj = build_fold_indirect_ref (sop);
> > > +			inform (DECL_SOURCE_LOCATION (obj),
> > > +				"pointed-to object declared here");
> > > +		      }
> > > +		    else
> > > +		      error_at (loc, "cast from %qT is not allowed",
> > 
> > Can this be more helpful as well, i.e. say because op is not the address of
> > an object of similar type?
> > 
> > Can we only get here if op is null, since we would have returned already for
> > non-constant op?
> > 
> > Jason
> > 
> 
> Hm, yes; I'd kept the error as it was because I wasn't sure what other
> kinds of trees might end up here, but I've done a fair amount of testing
> and I've only been able to reach here with null pointers: everything
> else gets caught earlier. I've updated the error message appropriately
> and documented this assumption with an assertion.
> 
> Bootstrapped + regtested on x86_64-pc-linux-gnu as above.
> 
> -- >8 --
> 
> This patch improves the errors given when casting from void* in C++26 to
> include the expected type if the types of the pointed-to objects were
> not similar. It also ensures (for all standard modes) that void* casts
> are checked even for DECL_ARTIFICIAL declarations, such as
> lifetime-extended temporaries, and is only ignored for cases where we
> know it's OK (e.g. source_location::current) or have no other choice
> (heap-allocated data).
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (is_std_source_location_current): New.
> 	(cxx_eval_constant_expression): Only ignore cast from void* for
> 	specific cases and improve other diagnostics.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/constexpr-cast4.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/constexpr.cc                          | 87 +++++++++++++++++---
>  gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C | 11 +++
>  2 files changed, 86 insertions(+), 12 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index 0f948db7c2d..d060e374cb3 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
>  	  && is_std_allocator_allocate (call->fundef->decl));
>  }
>  
> +/* Return true if FNDECL is std::source_location::current.  */
> +
> +static inline bool
> +is_std_source_location_current (tree fndecl)
> +{
> +  if (!decl_in_std_namespace_p (fndecl))
> +    return false;
> +
> +  tree name = DECL_NAME (fndecl);
> +  if (name == NULL_TREE || !id_equal (name, "current"))
> +    return false;
> +
> +  tree ctx = DECL_CONTEXT (fndecl);
> +  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
> +    return false;
> +
> +  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
> +  return name && id_equal (name, "source_location");
> +}
> +
> +/* Overload for the above taking constexpr_call*.  */
> +
> +static inline bool
> +is_std_source_location_current (const constexpr_call *call)
> +{
> +  return (call
> +	  && call->fundef
> +	  && is_std_source_location_current (call->fundef->decl));
> +}
> +
>  /* Return true if FNDECL is __dynamic_cast.  */
>  
>  static inline bool
> @@ -7850,33 +7880,66 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>  	if (TYPE_PTROB_P (type)
>  	    && TYPE_PTR_P (TREE_TYPE (op))
>  	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
> -	    /* Inside a call to std::construct_at or to
> -	       std::allocator<T>::{,de}allocate, we permit casting from void*
> +	    /* Inside a call to std::construct_at,
> +	       std::allocator<T>::{,de}allocate, or
> +	       std::source_location::current, we permit casting from void*
>  	       because that is compiler-generated code.  */
>  	    && !is_std_construct_at (ctx->call)
> -	    && !is_std_allocator_allocate (ctx->call))
> +	    && !is_std_allocator_allocate (ctx->call)
> +	    && !is_std_source_location_current (ctx->call))
>  	  {
>  	    /* Likewise, don't error when casting from void* when OP is
>  	       &heap uninit and similar.  */
>  	    tree sop = tree_strip_nop_conversions (op);
> -	    if (TREE_CODE (sop) == ADDR_EXPR
> -		&& VAR_P (TREE_OPERAND (sop, 0))
> -		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
> +	    tree decl = NULL_TREE;
> +	    if (TREE_CODE (sop) == ADDR_EXPR)
> +	      decl = TREE_OPERAND (sop, 0);
> +	    if (decl
> +		&& VAR_P (decl)
> +		&& DECL_ARTIFICIAL (decl)
> +		&& (DECL_NAME (decl) == heap_identifier
> +		    || DECL_NAME (decl) == heap_uninit_identifier
> +		    || DECL_NAME (decl) == heap_vec_identifier
> +		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
>  	      /* OK */;
>  	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
>  	       cv void" to a pointer-to-object type T unless P points to an
>  	       object whose type is similar to T.  */
> -	    else if (cxx_dialect > cxx23
> -		     && (sop = cxx_fold_indirect_ref (ctx, loc,
> -						      TREE_TYPE (type), sop)))
> +	    else if (cxx_dialect > cxx23)
>  	      {
> -		r = build1 (ADDR_EXPR, type, sop);
> -		break;
> +		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
> +		if (r)
> +		  {
> +		    r = build1 (ADDR_EXPR, type, r);
> +		    break;
> +		  }
> +		if (!ctx->quiet)
> +		  {
> +		    if (TREE_CODE (sop) == ADDR_EXPR)
> +		      {

A very minor point (sorry), but I think you want

  auto_diagnostic_group d;

here.  Not need to repost the patch only because of this.

> +			error_at (loc, "cast from %qT is not allowed because "
> +				  "pointed-to type %qT is not similar to %qT",
> +				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
> +				  TREE_TYPE (type));
> +			tree obj = build_fold_indirect_ref (sop);
> +			inform (DECL_SOURCE_LOCATION (obj),
> +				"pointed-to object declared here");
> +		      }

Marek


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] c++: Improve diagnostics for constexpr cast from void*
  2023-10-11 15:41     ` Marek Polacek
@ 2023-10-20  3:25       ` Jason Merrill
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2023-10-20  3:25 UTC (permalink / raw)
  To: Marek Polacek, Nathaniel Shead; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 11241 bytes --]

On 10/11/23 11:41, Marek Polacek wrote:
> On Wed, Oct 11, 2023 at 10:57:06AM +1100, Nathaniel Shead wrote:
>> On Mon, Oct 09, 2023 at 04:10:20PM -0400, Jason Merrill wrote:
>>> On 10/9/23 06:03, Nathaniel Shead wrote:
>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu with
>>>> GXX_TESTSUITE_STDS=98,11,14,17,20,23,26,impcx.
>>>>
>>>> -- >8 --
>>>>
>>>> This patch improves the errors given when casting from void* in C++26 to
>>>> include the expected type if the type of the pointed-to object was
>>>> not similar to the casted-to type.
>>>>
>>>> It also ensures (for all standard modes) that void* casts are checked
>>>> even for DECL_ARTIFICIAL declarations, such as lifetime-extended
>>>> temporaries, and is only ignored for cases where we know it's OK (heap
>>>> identifiers and source_location::current). This provides more accurate
>>>> diagnostics when using the pointer and ensures that some other casts
>>>> from void* are now correctly rejected.
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> 	* constexpr.cc (is_std_source_location_current): New.
>>>> 	(cxx_eval_constant_expression): Only ignore cast from void* for
>>>> 	specific cases and improve other diagnostics.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 	* g++.dg/cpp0x/constexpr-cast4.C: New test.
>>>>
>>>> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
>>>> ---
>>>>    gcc/cp/constexpr.cc                          | 83 +++++++++++++++++---
>>>>    gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C |  7 ++
>>>>    2 files changed, 78 insertions(+), 12 deletions(-)
>>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
>>>>
>>>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>>>> index 0f948db7c2d..f38d541a662 100644
>>>> --- a/gcc/cp/constexpr.cc
>>>> +++ b/gcc/cp/constexpr.cc
>>>> @@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
>>>>    	  && is_std_allocator_allocate (call->fundef->decl));
>>>>    }
>>>> +/* Return true if FNDECL is std::source_location::current.  */
>>>> +
>>>> +static inline bool
>>>> +is_std_source_location_current (tree fndecl)
>>>> +{
>>>> +  if (!decl_in_std_namespace_p (fndecl))
>>>> +    return false;
>>>> +
>>>> +  tree name = DECL_NAME (fndecl);
>>>> +  if (name == NULL_TREE || !id_equal (name, "current"))
>>>> +    return false;
>>>> +
>>>> +  tree ctx = DECL_CONTEXT (fndecl);
>>>> +  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
>>>> +    return false;
>>>> +
>>>> +  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
>>>> +  return name && id_equal (name, "source_location");
>>>> +}
>>>> +
>>>> +/* Overload for the above taking constexpr_call*.  */
>>>> +
>>>> +static inline bool
>>>> +is_std_source_location_current (const constexpr_call *call)
>>>> +{
>>>> +  return (call
>>>> +	  && call->fundef
>>>> +	  && is_std_source_location_current (call->fundef->decl));
>>>> +}
>>>> +
>>>>    /* Return true if FNDECL is __dynamic_cast.  */
>>>>    static inline bool
>>>> @@ -7850,33 +7880,62 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>>>>    	if (TYPE_PTROB_P (type)
>>>>    	    && TYPE_PTR_P (TREE_TYPE (op))
>>>>    	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
>>>> -	    /* Inside a call to std::construct_at or to
>>>> -	       std::allocator<T>::{,de}allocate, we permit casting from void*
>>>> +	    /* Inside a call to std::construct_at,
>>>> +	       std::allocator<T>::{,de}allocate, or
>>>> +	       std::source_location::current, we permit casting from void*
>>>>    	       because that is compiler-generated code.  */
>>>>    	    && !is_std_construct_at (ctx->call)
>>>> -	    && !is_std_allocator_allocate (ctx->call))
>>>> +	    && !is_std_allocator_allocate (ctx->call)
>>>> +	    && !is_std_source_location_current (ctx->call))
>>>>    	  {
>>>>    	    /* Likewise, don't error when casting from void* when OP is
>>>>    	       &heap uninit and similar.  */
>>>>    	    tree sop = tree_strip_nop_conversions (op);
>>>> -	    if (TREE_CODE (sop) == ADDR_EXPR
>>>> -		&& VAR_P (TREE_OPERAND (sop, 0))
>>>> -		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
>>>> +	    tree decl = NULL_TREE;
>>>> +	    if (TREE_CODE (sop) == ADDR_EXPR)
>>>> +	      decl = TREE_OPERAND (sop, 0);
>>>> +	    if (decl
>>>> +		&& VAR_P (decl)
>>>> +		&& DECL_ARTIFICIAL (decl)
>>>> +		&& (DECL_NAME (decl) == heap_identifier
>>>> +		    || DECL_NAME (decl) == heap_uninit_identifier
>>>> +		    || DECL_NAME (decl) == heap_vec_identifier
>>>> +		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
>>>>    	      /* OK */;
>>>>    	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
>>>>    	       cv void" to a pointer-to-object type T unless P points to an
>>>>    	       object whose type is similar to T.  */
>>>> -	    else if (cxx_dialect > cxx23
>>>> -		     && (sop = cxx_fold_indirect_ref (ctx, loc,
>>>> -						      TREE_TYPE (type), sop)))
>>>> +	    else if (cxx_dialect > cxx23)
>>>>    	      {
>>>> -		r = build1 (ADDR_EXPR, type, sop);
>>>> -		break;
>>>> +		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
>>>> +		if (r)
>>>> +		  {
>>>> +		    r = build1 (ADDR_EXPR, type, r);
>>>> +		    break;
>>>> +		  }
>>>> +		if (!ctx->quiet)
>>>> +		  {
>>>> +		    if (TREE_CODE (sop) == ADDR_EXPR)
>>>> +		      {
>>>> +			error_at (loc, "cast from %qT is not allowed because "
>>>> +				  "pointed-to type %qT is not similar to %qT",
>>>> +				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
>>>> +				  TREE_TYPE (type));
>>>> +			tree obj = build_fold_indirect_ref (sop);
>>>> +			inform (DECL_SOURCE_LOCATION (obj),
>>>> +				"pointed-to object declared here");
>>>> +		      }
>>>> +		    else
>>>> +		      error_at (loc, "cast from %qT is not allowed",
>>>
>>> Can this be more helpful as well, i.e. say because op is not the address of
>>> an object of similar type?
>>>
>>> Can we only get here if op is null, since we would have returned already for
>>> non-constant op?
>>>
>>> Jason
>>>
>>
>> Hm, yes; I'd kept the error as it was because I wasn't sure what other
>> kinds of trees might end up here, but I've done a fair amount of testing
>> and I've only been able to reach here with null pointers: everything
>> else gets caught earlier. I've updated the error message appropriately
>> and documented this assumption with an assertion.
>>
>> Bootstrapped + regtested on x86_64-pc-linux-gnu as above.
>>
>> -- >8 --
>>
>> This patch improves the errors given when casting from void* in C++26 to
>> include the expected type if the types of the pointed-to objects were
>> not similar. It also ensures (for all standard modes) that void* casts
>> are checked even for DECL_ARTIFICIAL declarations, such as
>> lifetime-extended temporaries, and is only ignored for cases where we
>> know it's OK (e.g. source_location::current) or have no other choice
>> (heap-allocated data).
>>
>> gcc/cp/ChangeLog:
>>
>> 	* constexpr.cc (is_std_source_location_current): New.
>> 	(cxx_eval_constant_expression): Only ignore cast from void* for
>> 	specific cases and improve other diagnostics.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/cpp0x/constexpr-cast4.C: New test.
>>
>> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
>> ---
>>   gcc/cp/constexpr.cc                          | 87 +++++++++++++++++---
>>   gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C | 11 +++
>>   2 files changed, 86 insertions(+), 12 deletions(-)
>>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
>>
>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>> index 0f948db7c2d..d060e374cb3 100644
>> --- a/gcc/cp/constexpr.cc
>> +++ b/gcc/cp/constexpr.cc
>> @@ -2301,6 +2301,36 @@ is_std_allocator_allocate (const constexpr_call *call)
>>   	  && is_std_allocator_allocate (call->fundef->decl));
>>   }
>>   
>> +/* Return true if FNDECL is std::source_location::current.  */
>> +
>> +static inline bool
>> +is_std_source_location_current (tree fndecl)
>> +{
>> +  if (!decl_in_std_namespace_p (fndecl))
>> +    return false;
>> +
>> +  tree name = DECL_NAME (fndecl);
>> +  if (name == NULL_TREE || !id_equal (name, "current"))
>> +    return false;
>> +
>> +  tree ctx = DECL_CONTEXT (fndecl);
>> +  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
>> +    return false;
>> +
>> +  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
>> +  return name && id_equal (name, "source_location");
>> +}
>> +
>> +/* Overload for the above taking constexpr_call*.  */
>> +
>> +static inline bool
>> +is_std_source_location_current (const constexpr_call *call)
>> +{
>> +  return (call
>> +	  && call->fundef
>> +	  && is_std_source_location_current (call->fundef->decl));
>> +}
>> +
>>   /* Return true if FNDECL is __dynamic_cast.  */
>>   
>>   static inline bool
>> @@ -7850,33 +7880,66 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
>>   	if (TYPE_PTROB_P (type)
>>   	    && TYPE_PTR_P (TREE_TYPE (op))
>>   	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
>> -	    /* Inside a call to std::construct_at or to
>> -	       std::allocator<T>::{,de}allocate, we permit casting from void*
>> +	    /* Inside a call to std::construct_at,
>> +	       std::allocator<T>::{,de}allocate, or
>> +	       std::source_location::current, we permit casting from void*
>>   	       because that is compiler-generated code.  */
>>   	    && !is_std_construct_at (ctx->call)
>> -	    && !is_std_allocator_allocate (ctx->call))
>> +	    && !is_std_allocator_allocate (ctx->call)
>> +	    && !is_std_source_location_current (ctx->call))
>>   	  {
>>   	    /* Likewise, don't error when casting from void* when OP is
>>   	       &heap uninit and similar.  */
>>   	    tree sop = tree_strip_nop_conversions (op);
>> -	    if (TREE_CODE (sop) == ADDR_EXPR
>> -		&& VAR_P (TREE_OPERAND (sop, 0))
>> -		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
>> +	    tree decl = NULL_TREE;
>> +	    if (TREE_CODE (sop) == ADDR_EXPR)
>> +	      decl = TREE_OPERAND (sop, 0);
>> +	    if (decl
>> +		&& VAR_P (decl)
>> +		&& DECL_ARTIFICIAL (decl)
>> +		&& (DECL_NAME (decl) == heap_identifier
>> +		    || DECL_NAME (decl) == heap_uninit_identifier
>> +		    || DECL_NAME (decl) == heap_vec_identifier
>> +		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
>>   	      /* OK */;
>>   	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
>>   	       cv void" to a pointer-to-object type T unless P points to an
>>   	       object whose type is similar to T.  */
>> -	    else if (cxx_dialect > cxx23
>> -		     && (sop = cxx_fold_indirect_ref (ctx, loc,
>> -						      TREE_TYPE (type), sop)))
>> +	    else if (cxx_dialect > cxx23)
>>   	      {
>> -		r = build1 (ADDR_EXPR, type, sop);
>> -		break;
>> +		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
>> +		if (r)
>> +		  {
>> +		    r = build1 (ADDR_EXPR, type, r);
>> +		    break;
>> +		  }
>> +		if (!ctx->quiet)
>> +		  {
>> +		    if (TREE_CODE (sop) == ADDR_EXPR)
>> +		      {
> 
> A very minor point (sorry), but I think you want
> 
>    auto_diagnostic_group d;
> 
> here.  Not need to repost the patch only because of this.

I'm applying this patch with that tweak and also adjusting "not allowed" 
to "not allowed in a constant expression".  Thanks!


[-- Attachment #2: 0001-c-Improve-diagnostics-for-constexpr-cast-from-void.patch --]
[-- Type: text/x-patch, Size: 6619 bytes --]

From 7ca718baf62da20037d8162b312f0431290ed642 Mon Sep 17 00:00:00 2001
From: Nathaniel Shead <nathanieloshead@gmail.com>
Date: Wed, 11 Oct 2023 10:57:06 +1100
Subject: [PATCH] c++: Improve diagnostics for constexpr cast from void*
To: gcc-patches@gcc.gnu.org

This patch improves the errors given when casting from void* in C++26 to
include the expected type if the types of the pointed-to objects were
not similar. It also ensures (for all standard modes) that void* casts
are checked even for DECL_ARTIFICIAL declarations, such as
lifetime-extended temporaries, and is only ignored for cases where we
know it's OK (e.g. source_location::current) or have no other choice
(heap-allocated data).

gcc/cp/ChangeLog:

	* constexpr.cc (is_std_source_location_current): New.
	(cxx_eval_constant_expression): Only ignore cast from void* for
	specific cases and improve other diagnostics.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/constexpr-cast4.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Marek Polacek  <polacek@redhat.com>
Reviewed-by: Jason Merrill <jason@redhat.com>
---
 gcc/cp/constexpr.cc                          | 91 +++++++++++++++++---
 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C | 11 +++
 2 files changed, 90 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C

diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index aa121b0edbc..6e84c2d77bb 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2309,6 +2309,36 @@ is_std_allocator_allocate (const constexpr_call *call)
 	  && is_std_allocator_allocate (call->fundef->decl));
 }
 
+/* Return true if FNDECL is std::source_location::current.  */
+
+static inline bool
+is_std_source_location_current (tree fndecl)
+{
+  if (!decl_in_std_namespace_p (fndecl))
+    return false;
+
+  tree name = DECL_NAME (fndecl);
+  if (name == NULL_TREE || !id_equal (name, "current"))
+    return false;
+
+  tree ctx = DECL_CONTEXT (fndecl);
+  if (ctx == NULL_TREE || !CLASS_TYPE_P (ctx) || !TYPE_MAIN_DECL (ctx))
+    return false;
+
+  name = DECL_NAME (TYPE_MAIN_DECL (ctx));
+  return name && id_equal (name, "source_location");
+}
+
+/* Overload for the above taking constexpr_call*.  */
+
+static inline bool
+is_std_source_location_current (const constexpr_call *call)
+{
+  return (call
+	  && call->fundef
+	  && is_std_source_location_current (call->fundef->decl));
+}
+
 /* Return true if FNDECL is __dynamic_cast.  */
 
 static inline bool
@@ -7938,33 +7968,70 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 	if (TYPE_PTROB_P (type)
 	    && TYPE_PTR_P (TREE_TYPE (op))
 	    && VOID_TYPE_P (TREE_TYPE (TREE_TYPE (op)))
-	    /* Inside a call to std::construct_at or to
-	       std::allocator<T>::{,de}allocate, we permit casting from void*
+	    /* Inside a call to std::construct_at,
+	       std::allocator<T>::{,de}allocate, or
+	       std::source_location::current, we permit casting from void*
 	       because that is compiler-generated code.  */
 	    && !is_std_construct_at (ctx->call)
-	    && !is_std_allocator_allocate (ctx->call))
+	    && !is_std_allocator_allocate (ctx->call)
+	    && !is_std_source_location_current (ctx->call))
 	  {
 	    /* Likewise, don't error when casting from void* when OP is
 	       &heap uninit and similar.  */
 	    tree sop = tree_strip_nop_conversions (op);
-	    if (TREE_CODE (sop) == ADDR_EXPR
-		&& VAR_P (TREE_OPERAND (sop, 0))
-		&& DECL_ARTIFICIAL (TREE_OPERAND (sop, 0)))
+	    tree decl = NULL_TREE;
+	    if (TREE_CODE (sop) == ADDR_EXPR)
+	      decl = TREE_OPERAND (sop, 0);
+	    if (decl
+		&& VAR_P (decl)
+		&& DECL_ARTIFICIAL (decl)
+		&& (DECL_NAME (decl) == heap_identifier
+		    || DECL_NAME (decl) == heap_uninit_identifier
+		    || DECL_NAME (decl) == heap_vec_identifier
+		    || DECL_NAME (decl) == heap_vec_uninit_identifier))
 	      /* OK */;
 	    /* P2738 (C++26): a conversion from a prvalue P of type "pointer to
 	       cv void" to a pointer-to-object type T unless P points to an
 	       object whose type is similar to T.  */
-	    else if (cxx_dialect > cxx23
-		     && (sop = cxx_fold_indirect_ref (ctx, loc,
-						      TREE_TYPE (type), sop)))
+	    else if (cxx_dialect > cxx23)
 	      {
-		r = build1 (ADDR_EXPR, type, sop);
-		break;
+		r = cxx_fold_indirect_ref (ctx, loc, TREE_TYPE (type), sop);
+		if (r)
+		  {
+		    r = build1 (ADDR_EXPR, type, r);
+		    break;
+		  }
+		if (!ctx->quiet)
+		  {
+		    if (TREE_CODE (sop) == ADDR_EXPR)
+		      {
+			auto_diagnostic_group d;
+			error_at (loc, "cast from %qT is not allowed in a "
+				  "constant expression because "
+				  "pointed-to type %qT is not similar to %qT",
+				  TREE_TYPE (op), TREE_TYPE (TREE_TYPE (sop)),
+				  TREE_TYPE (type));
+			tree obj = build_fold_indirect_ref (sop);
+			inform (DECL_SOURCE_LOCATION (obj),
+				"pointed-to object declared here");
+		      }
+		    else
+		      {
+			gcc_assert (integer_zerop (sop));
+			error_at (loc, "cast from %qT is not allowed in a "
+				  "constant expression because "
+				  "%qE does not point to an object",
+				  TREE_TYPE (op), oldop);
+		      }
+		  }
+		*non_constant_p = true;
+		return t;
 	      }
 	    else
 	      {
 		if (!ctx->quiet)
-		  error_at (loc, "cast from %qT is not allowed",
+		  error_at (loc, "cast from %qT is not allowed in a "
+			    "constant expression before C++26",
 			    TREE_TYPE (op));
 		*non_constant_p = true;
 		return t;
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
new file mode 100644
index 00000000000..884b6a53e3b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-cast4.C
@@ -0,0 +1,11 @@
+// { dg-do compile { target c++11 } }
+
+constexpr int&& r = 1 + 2;  // { dg-message "pointed-to object declared here" "" { target c++26 } }
+constexpr void* vpr = &r;
+constexpr int* pi = static_cast<int*>(vpr);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+constexpr float* pf = static_cast<float*>(vpr);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+// { dg-error "cast from .void\\*. is not allowed in a constant expression because pointed-to type .int. is not similar to .float." "" { target c++26 } .-1 }
+
+constexpr void* vnp = nullptr;
+constexpr int* pi2 = static_cast<int*>(vnp);  // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } }
+// { dg-error "cast from .void\\*. is not allowed in a constant expression because .vnp. does not point to an object" "" { target c++26 } .-1 }
-- 
2.39.3


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-10-20  3:25 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-09 10:03 [PATCH] c++: Improve diagnostics for constexpr cast from void* Nathaniel Shead
2023-10-09 20:10 ` Jason Merrill
2023-10-10 23:57   ` [PATCH v2] " Nathaniel Shead
2023-10-11 15:41     ` Marek Polacek
2023-10-20  3:25       ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).