From patchwork Tue Oct 10 23:57:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 1846183 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=g4O3hCR2; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S4tBJ3sxWz1yq4 for ; Wed, 11 Oct 2023 10:57:41 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 617693858D35 for ; Tue, 10 Oct 2023 23:57:39 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com [IPv6:2607:f8b0:4864:20::1036]) by sourceware.org (Postfix) with ESMTPS id 2B0983858D35 for ; Tue, 10 Oct 2023 23:57:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2B0983858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x1036.google.com with SMTP id 98e67ed59e1d1-27752a1e184so5006741a91.3 for ; Tue, 10 Oct 2023 16:57:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696982232; x=1697587032; darn=gcc.gnu.org; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:from:to:cc:subject:date:message-id:reply-to; bh=fdmqgovtiEoARcVfNZeNF6HHd1hoRPDBzk0V1SocmRw=; b=g4O3hCR26pDjvcEasxF8i32mXdaN4lE/gl5lgURxq8TZ+xxF+WJFN20u8C4FDCoZJw 727Y26UyDxxhXJGoqHvc3ojoHhMXYrWcMnsBVnJacuNV7stsW0J7Qe9CP2jRTNSrrGYq kEL7o9wqtZd/5p1F1yQRsRZg64owvWU2nLezCBgKLv9nes3I/uoBWGoHxADdK6B1/zGw pAhI8gVxo6EPbPIDQZa3tZcSAw+zsC6ip7bFmu/YxwohB+mdVDQfu51xZbNcBIinmd5p REX5Zkh34vYTR7yhgFV4jsk93lBrq84uOYNpXKA2W21tL6RpOjhCQCJwJG8nRCWRWnAG Mtng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696982232; x=1697587032; h=in-reply-to:content-disposition:mime-version:references:subject:cc :to:from:date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=fdmqgovtiEoARcVfNZeNF6HHd1hoRPDBzk0V1SocmRw=; b=nrmFscv7hC7Bh7v6xpza7kpNdwbeuuly4uXutGMXSoYbWpHgH5aHGuopc/xgmg3ofM +ik6mxf7+dcHl4w1BeO3ikCR+QkDTQz6jRE6x+3ujkdJ1NBCOhgQvSdZT/sq9kQiMvae TjZ24WLPfj+yUpq380Yy189MFlwkCSZQ1IVLqpbquINW+nW9EmYEzLHaMOVXDIRqXZ8D 2hs/nriqxUuP+GQI+ToSArt/WnzE4MMwO0q0rW9KEc4G5YltnzOukDYVzqVopt3tlOwn SXOpWsWhSaqFswlMlZxTvSHkdVZG8eiHRxXL721d0DTrF9oo6q4qVPSeSpUnw/Nu/CCg BM3w== X-Gm-Message-State: AOJu0YzyEofzxtKAn4d9uT/aK+alQ2yOH/TUCc86Uev2gWHhS6Du9WC3 hbz9VZo1yaNQL9Yf8a8KJJz8h34bYxs= X-Google-Smtp-Source: AGHT+IF/zi7DxhbPhn2xD0bwnSiJZc8RMNIhOBDLQDYzg1jX8EQmgR6GpUjrCDavOG/d/OW/Nu2FLg== X-Received: by 2002:a17:90b:3686:b0:268:1354:7b03 with SMTP id mj6-20020a17090b368600b0026813547b03mr19030504pjb.12.1696982231705; Tue, 10 Oct 2023 16:57:11 -0700 (PDT) Received: from Thaum. (124-150-88-161.tpgi.com.au. [124.150.88.161]) by smtp.gmail.com with ESMTPSA id gi9-20020a17090b110900b00277246e857esm12463889pjb.23.2023.10.10.16.57.09 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 16:57:11 -0700 (PDT) Message-ID: <6525e4d7.170a0220.2ad8c.28d8@mx.google.com> X-Google-Original-Message-ID: Date: Wed, 11 Oct 2023 10:57:06 +1100 From: Nathaniel Shead To: Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [PATCH v2] c++: Improve diagnostics for constexpr cast from void* References: <6523cffe.170a0220.653b.72b9@mx.google.com> <6e35813a-537d-4af2-8675-3b2caf104d69@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6e35813a-537d-4af2-8675-3b2caf104d69@redhat.com> X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org 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) + { + 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(vpr); // { dg-error "cast from .void\\*. is not allowed" "" { target c++23_down } } +constexpr float* pf = static_cast(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(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 }