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 >>>> --- >>>> 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::{,de}allocate, we permit casting from void* >>>> + /* Inside a call to std::construct_at, >>>> + std::allocator::{,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 >> --- >> 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::{,de}allocate, we permit casting from void* >> + /* Inside a call to std::construct_at, >> + std::allocator::{,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!