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)
+                     {
+                       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

Reply via email to