From: Jason Merrill <jason@redhat.com>
To: Marek Polacek <polacek@redhat.com>,
Nathaniel Shead <nathanieloshead@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH v2] c++: Improve diagnostics for constexpr cast from void*
Date: Thu, 19 Oct 2023 23:25:03 -0400 [thread overview]
Message-ID: <c699ed1a-0c2b-4b68-bc32-7c9f12e766b7@redhat.com> (raw)
In-Reply-To: <ZSbCGijGOEyMpmJs@redhat.com>
[-- 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
prev parent reply other threads:[~2023-10-20 3:25 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-09 10:03 [PATCH] " 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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c699ed1a-0c2b-4b68-bc32-7c9f12e766b7@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=nathanieloshead@gmail.com \
--cc=polacek@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).