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 <nathanielosh...@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 <nathanielosh...@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
