From patchwork Mon Apr 15 04:49:35 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathaniel Shead X-Patchwork-Id: 1923559 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=e+Be6Yy+; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; 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 [IPv6:2620:52:3:1:0:246e:9693:128c]) (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 4VHvqW6kcGz1yXv for ; Mon, 15 Apr 2024 14:50:13 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B5B3F3858C98 for ; Mon, 15 Apr 2024 04:50:04 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) by sourceware.org (Postfix) with ESMTPS id 174593858D34 for ; Mon, 15 Apr 2024 04:49:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 174593858D34 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 174593858D34 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::52e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713156585; cv=none; b=D11R9AJ68965xfhcIRlU5aunSt2ueDE+LFfTNVub+gFwtmxE7bEzDxeeQ56HRKlg/1eaHnjmpNvwsoTzbqxkEyGb2ClgMBy9R0IVElg91bMygyx0eEQGjclcm/5/Qvt9dD4lvvdO/p0ZwdtJRTCv2MOZiqJ0bt9JSozvjgXaL4c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713156585; c=relaxed/simple; bh=VFw5L1z3MVAzsTuf9ApoAm8YoBAOqCBpxom/xB2DnDU=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=p65wxaviwYyxtrg7a3J9FioFUKCzeLNBbZv5Ae1XmVRZdjfDu/np4ExJh/CJZVz9hjhFyafwx3K1avgMyMiQMSwd38yjH8jJbOIYhTap3+cHwMuHkQoPoo5BxZZXRKHEyY8jxiEyiWgg2dT6b7xPAHWD9ILHREUkBfuMcqVVDy4= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52e.google.com with SMTP id 41be03b00d2f7-5f415fd71f8so2181610a12.3 for ; Sun, 14 Apr 2024 21:49:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713156581; x=1713761381; 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=gi/aY6g93uO9xz7d9sf9ciaraoXiak06+oXijBLnh8w=; b=e+Be6Yy+172uzJx2JvzsC/RZ/GAGpFkBxtlgbQYDECz6tGukbfwIAmJlNl6adILUBw +yXvlnh3ahX8b0VXD0UfLYyMWHGsyFg22YC1GHJttblLFtznD6Ad734UOZ8ORuYCDjcb q77G7ZYLl5GuQrViRTq219F2b6aB7q43P6fPgmmbxvrzZRdaLUD5ScZ2Xa8aTd8x11RE 6mVbDo2RV7FIskFtegWx1oiO6wcghl4TZfzKhkhiVLylHsRv68djIfI4+rPMNOIYn5H7 Xy384IzP1jUIL5Q01FeYyTz7Am3WTsIlTC/27LeWDxgq3au7vaiTtEayhcmNB+3vnEIt J5EA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713156581; x=1713761381; 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=gi/aY6g93uO9xz7d9sf9ciaraoXiak06+oXijBLnh8w=; b=FbiYB5sEZW/5nKjRgqaagn5qoBCjHSeIFKTpwhv8slLJGEonVPid9u0gA1juIVN83G U5mteBQUvz9qpcfH5x98Tm0Ko3HQAz7nROGBx8tDBFK9V/Hs8zruarB5VfLqfLSFZFWE g3YMHdTTOp0Vvmwh5Pc+o/gDWVKx/oaMOCM2P8lPGEBD21vehkVLT0aDbpqc8V4c9MAm MrXO/ao0ZrCmqp6Pqt2iAjx79q6HsojK9SW5H0n1sS7WTMvbQdvv6a5ftOQBpmxvTYpQ Wzc/tsspVN6NJTVPulDUx8XmlIHGN7uK/QOwqHz+0uIcwTe+jJD5s2i8GGpGTDkOgNtc NzDA== X-Gm-Message-State: AOJu0Yw1zoLdczof90G1qHnPgz/m3NRyBJHFA69qukWatORDY2HXMGE+ QaaSxRcmnlwQXGg5MfSfat4d0RIbyEPvBLzoat6oyDww5UAhUp8gs8a/XQ== X-Google-Smtp-Source: AGHT+IEfc4OPlUfUAIZVG5Lx4qb6ION7bh0VxM6TcljCiECtVf2wPkocn9lWdyxametlRhrbx3mwlw== X-Received: by 2002:a05:6a20:7490:b0:1a7:42cd:b207 with SMTP id p16-20020a056a20749000b001a742cdb207mr8768109pzd.19.1713156580822; Sun, 14 Apr 2024 21:49:40 -0700 (PDT) Received: from Thaum. (121-44-11-123.tpgi.com.au. [121.44.11.123]) by smtp.gmail.com with ESMTPSA id y15-20020a17090322cf00b001e40898e9acsm6924400plg.276.2024.04.14.21.49.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 14 Apr 2024 21:49:40 -0700 (PDT) Message-ID: <661cb1e4.170a0220.e8efc.335d@mx.google.com> X-Google-Original-Message-ID: Date: Mon, 15 Apr 2024 14:49:35 +1000 From: Nathaniel Shead To: gcc-patches@gcc.gnu.org Cc: Jason Merrill , Nathan Sidwell , Patrick Palka Subject: [PATCH v2 1/2] c++: Standardise errors for module_may_redeclare References: <66014130.170a0220.d7c40.0e9b@mx.google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <66014130.170a0220.d7c40.0e9b@mx.google.com> X-Spam-Status: No, score=-11.1 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, URIBL_BLACK 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 I took another look at this patch and have split it into two, one (this one) to standardise the error messages used and prepare 'module_may_redeclare' for use with temploid friends, and another followup patch to actually handle them correctly. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- Currently different places calling 'module_may_redeclare' all emit very similar but slightly different error messages, and handle different kinds of declarations differently. This patch makes the function perform its own error messages so that they're all in one place, and prepares it for use with temploid friends (PR c++/114275). gcc/cp/ChangeLog: * cp-tree.h (module_may_redeclare): Add default parameter. * decl.cc (duplicate_decls): Don't emit errors for failed module_may_redeclare. (xref_tag): Likewise. (start_enum): Likewise. * semantics.cc (begin_class_definition): Likewise. * module.cc (module_may_redeclare): Clean up logic. Emit error messages on failure. gcc/testsuite/ChangeLog: * g++.dg/modules/enum-12.C: Update error message. * g++.dg/modules/friend-5_b.C: Likewise. * g++.dg/modules/shadow-1_b.C: Likewise. Signed-off-by: Nathaniel Shead --- gcc/cp/cp-tree.h | 2 +- gcc/cp/decl.cc | 28 +---- gcc/cp/module.cc | 120 ++++++++++++++-------- gcc/cp/semantics.cc | 6 +- gcc/testsuite/g++.dg/modules/enum-12.C | 2 +- gcc/testsuite/g++.dg/modules/friend-5_b.C | 2 +- gcc/testsuite/g++.dg/modules/shadow-1_b.C | 5 +- 7 files changed, 89 insertions(+), 76 deletions(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 1dbb577a38d..faa7a0052a5 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7401,7 +7401,7 @@ inline bool module_exporting_p () extern module_state *get_module (tree name, module_state *parent = NULL, bool partition = false); -extern bool module_may_redeclare (tree decl); +extern bool module_may_redeclare (tree olddecl, tree newdecl = NULL); extern bool module_global_init_needed (); extern bool module_determine_import_inits (); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 65ab64885ff..aa66da4829d 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -2279,18 +2279,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) && TREE_CODE (olddecl) != NAMESPACE_DECL && !hiding) { - if (!module_may_redeclare (olddecl)) - { - if (DECL_ARTIFICIAL (olddecl)) - error ("declaration %qD conflicts with builtin", newdecl); - else - { - error ("declaration %qD conflicts with import", newdecl); - inform (olddecl_loc, "import declared %q#D here", olddecl); - } - - return error_mark_node; - } + if (!module_may_redeclare (olddecl, newdecl)) + return error_mark_node; tree not_tmpl = STRIP_TEMPLATE (olddecl); if (DECL_LANG_SPECIFIC (not_tmpl) @@ -16620,12 +16610,7 @@ xref_tag (enum tag_types tag_code, tree name, { tree decl = TYPE_NAME (t); if (!module_may_redeclare (decl)) - { - auto_diagnostic_group d; - error ("cannot declare %qD in a different module", decl); - inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); - return error_mark_node; - } + return error_mark_node; tree not_tmpl = STRIP_TEMPLATE (decl); if (DECL_LANG_SPECIFIC (not_tmpl) @@ -16973,12 +16958,7 @@ start_enum (tree name, tree enumtype, tree underlying_type, { tree decl = TYPE_NAME (enumtype); if (!module_may_redeclare (decl)) - { - auto_diagnostic_group d; - error ("cannot declare %qD in different module", decl); - inform (DECL_SOURCE_LOCATION (decl), "previously declared here"); - enumtype = error_mark_node; - } + enumtype = error_mark_node; else set_instantiating_module (decl); } diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 001430a4a8f..e2d2910ae48 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -18992,11 +18992,15 @@ get_importing_module (tree decl, bool flexible) return module->mod; } -/* Is it permissible to redeclare DECL. */ +/* Is it permissible to redeclare OLDDECL with NEWDECL. + + If NEWDECL is NULL, assumes that OLDDECL will be redeclared using + the current scope's module and attachment. */ bool -module_may_redeclare (tree decl) +module_may_redeclare (tree olddecl, tree newdecl) { + tree decl = olddecl; for (;;) { tree ctx = CP_DECL_CONTEXT (decl); @@ -19010,58 +19014,94 @@ module_may_redeclare (tree decl) decl = TYPE_NAME (ctx); } - tree not_tmpl = STRIP_TEMPLATE (decl); - int use_tpl = 0; - if (node_template_info (not_tmpl, use_tpl) && use_tpl) + if (node_template_info (STRIP_TEMPLATE (decl), use_tpl) && use_tpl) // Specializations of any kind can be redeclared anywhere. // FIXME: Should we be checking this in more places on the scope chain? return true; - if (!DECL_LANG_SPECIFIC (not_tmpl) || !DECL_MODULE_ATTACH_P (not_tmpl)) - // Decl is attached to global module. Current scope needs to be too. - return !module_attach_p (); + module_state *old_mod = (*modules)[0]; + module_state *new_mod = old_mod; - module_state *me = (*modules)[0]; - module_state *them = me; + tree old_origin = get_originating_module_decl (decl); + tree old_inner = STRIP_TEMPLATE (old_origin); + bool olddecl_attached_p = (DECL_LANG_SPECIFIC (old_inner) + && DECL_MODULE_ATTACH_P (old_inner)); + if (DECL_LANG_SPECIFIC (old_inner) && DECL_MODULE_IMPORT_P (old_inner)) + { + unsigned index = import_entity_index (old_origin); + old_mod = import_entity_module (index); + } - if (DECL_LANG_SPECIFIC (not_tmpl) && DECL_MODULE_IMPORT_P (not_tmpl)) + bool newdecl_attached_p = module_attach_p (); + if (newdecl) { - /* We can be given the TEMPLATE_RESULT. We want the - TEMPLATE_DECL. */ - int use_tpl = -1; - if (tree ti = node_template_info (decl, use_tpl)) + tree new_origin = get_originating_module_decl (newdecl); + tree new_inner = STRIP_TEMPLATE (new_origin); + newdecl_attached_p = (DECL_LANG_SPECIFIC (new_inner) + && DECL_MODULE_ATTACH_P (new_inner)); + if (DECL_LANG_SPECIFIC (new_inner) && DECL_MODULE_IMPORT_P (new_inner)) { - tree tmpl = TI_TEMPLATE (ti); - if (use_tpl == 2) - { - /* A partial specialization. Find that specialization's - template_decl. */ - for (tree list = DECL_TEMPLATE_SPECIALIZATIONS (tmpl); - list; list = TREE_CHAIN (list)) - if (DECL_TEMPLATE_RESULT (TREE_VALUE (list)) == decl) - { - decl = TREE_VALUE (list); - break; - } - } - else if (DECL_TEMPLATE_RESULT (tmpl) == decl) - decl = tmpl; + unsigned index = import_entity_index (new_origin); + new_mod = import_entity_module (index); } - unsigned index = import_entity_index (decl); - them = import_entity_module (index); } - // Decl is attached to named module. Current scope needs to be - // attaching to the same module. - if (!module_attach_p ()) - return false; + /* Module attachment needs to match. */ + if (olddecl_attached_p == newdecl_attached_p) + { + if (!olddecl_attached_p) + /* Both are GM entities, OK. */ + return true; - // Both attached to named module. - if (me == them) - return true; + if (new_mod == old_mod + || (new_mod && get_primary (new_mod) == get_primary (old_mod))) + /* Both attached to same named module, OK. */ + return true; + } + + /* Attached to different modules, error. */ + decl = newdecl ? newdecl : olddecl; + location_t loc = newdecl ? DECL_SOURCE_LOCATION (newdecl) : input_location; + if (DECL_ARTIFICIAL (olddecl) && !DECL_IMPLICIT_TYPEDEF_P (olddecl)) + error_at (loc, "declaration %qD conflicts with builtin", decl); + else if (DECL_LANG_SPECIFIC (old_inner) && DECL_MODULE_IMPORT_P (old_inner)) + { + auto_diagnostic_group d; + if (newdecl_attached_p) + error_at (loc, "redeclaring %qD in module %qs conflicts with import", + decl, new_mod->get_flatname ()); + else + error_at (loc, "redeclaring %qD in global module conflicts with import", + decl); - return me && get_primary (them) == get_primary (me); + if (olddecl_attached_p) + inform (DECL_SOURCE_LOCATION (olddecl), + "import declared attached to module %qs", + old_mod->get_flatname ()); + else + inform (DECL_SOURCE_LOCATION (olddecl), + "import declared in global module"); + } + else + { + auto_diagnostic_group d; + if (newdecl_attached_p) + error_at (loc, "conflicting declaration of %qD in module %qs", + decl, new_mod->get_flatname ()); + else + error_at (loc, "conflicting declaration of %qD in global module", + decl); + + if (olddecl_attached_p) + inform (DECL_SOURCE_LOCATION (olddecl), + "previously declared in module %qs", + old_mod->get_flatname ()); + else + inform (DECL_SOURCE_LOCATION (olddecl), + "previously declared in global module"); + } + return false; } /* DECL is being created by this TU. Record it came from here. We diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 02c7c1bf5a4..2dde65a970b 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -3777,11 +3777,7 @@ begin_class_definition (tree t) if (modules_p ()) { if (!module_may_redeclare (TYPE_NAME (t))) - { - error ("cannot declare %qD in a different module", TYPE_NAME (t)); - inform (DECL_SOURCE_LOCATION (TYPE_NAME (t)), "declared here"); - return error_mark_node; - } + return error_mark_node; set_instantiating_module (TYPE_NAME (t)); set_defining_module (TYPE_NAME (t)); } diff --git a/gcc/testsuite/g++.dg/modules/enum-12.C b/gcc/testsuite/g++.dg/modules/enum-12.C index 57eeb85d92a..019c3da4218 100644 --- a/gcc/testsuite/g++.dg/modules/enum-12.C +++ b/gcc/testsuite/g++.dg/modules/enum-12.C @@ -4,7 +4,7 @@ export module foo; namespace std { - enum class align_val_t : decltype(sizeof(int)) {}; // { dg-error "different module" } + enum class align_val_t : decltype(sizeof(int)) {}; // { dg-error "conflicting declaration" } } // { dg-prune-output "not writing module" } diff --git a/gcc/testsuite/g++.dg/modules/friend-5_b.C b/gcc/testsuite/g++.dg/modules/friend-5_b.C index f043d7a340d..6b561265155 100644 --- a/gcc/testsuite/g++.dg/modules/friend-5_b.C +++ b/gcc/testsuite/g++.dg/modules/friend-5_b.C @@ -4,7 +4,7 @@ export module bar; import foo; -class B { // { dg-error "in a different module" } +class B { // { dg-error "conflicts with import" } B() { object.value = 42; } A object; }; diff --git a/gcc/testsuite/g++.dg/modules/shadow-1_b.C b/gcc/testsuite/g++.dg/modules/shadow-1_b.C index 646381237ac..7f6a3182998 100644 --- a/gcc/testsuite/g++.dg/modules/shadow-1_b.C +++ b/gcc/testsuite/g++.dg/modules/shadow-1_b.C @@ -1,8 +1,5 @@ // { dg-additional-options -fmodules-ts } import shadow; -// unfortunately not the exact same diagnostic in both cases :( - void stat (); // { dg-error "conflicts with import" } - -struct stat {}; // { dg-error "in a different module" } +struct stat {}; // { dg-error "conflicts with import" }