From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 3DC333858D20 for ; Wed, 17 Apr 2024 15:13:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3DC333858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 3DC333858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713366804; cv=none; b=OXyS7PB7SB5ZQP0UVrPp1bjbld2j6/ce4LEadIABxBgbatM8fz3ZhILiy9jl8jfS2b/zJqzJPXr816kqb9ubpn+7CLy3ClQpqYeLQtc2lVi4YfYgV7E94uSZmAcJhpHSiAxXAXstR+FB8iGmB1t04+xXQelCFDwSPsnjmaA7//c= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713366804; c=relaxed/simple; bh=nDVnJlbqPa7bQMlddWdNOUoZnCiq+FLTxbOiffB7uHw=; h=DKIM-Signature:From:Date:To:Subject:Message-ID:MIME-Version; b=A4HsrGLBxC+smmpOLD38ZBi/CdPgPEV/4g6XhBr/92XBzVC+GsPbYpo8i/yoD6O0A10I5C5k2vLy/ZGoNdYLa9AjeGaExRhLosXpADP+gQj/lUC2w+ak0MV+sK2hP/JPdWpMdqR/w/fc9VRAAkQSeeAaEIJyOXCZwY+xkR9QMqM= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713366800; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=DZg7n1wX1pk52QGJxf9KI7U4cqI2FNBfSmPHV2lo4hg=; b=BQOzMUaMBrfEiLKw32W03hICld9Q3b7Tj7LXa1yPVxE7a1w2z/dtqCARlLuTAAz32ZrNIQ s2agl5HsaHJPMGQ7mfLAKZZ1G8OXU74oWQNkAT+gVlzNA2LGto00ggror6NtImew4rzzFt oBsLDoEO2ptjA+Y4BghNsyA7E8nCQeo= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-370-Nn8Mw6sHNA-ris6DQcw_5g-1; Wed, 17 Apr 2024 11:13:18 -0400 X-MC-Unique: Nn8Mw6sHNA-ris6DQcw_5g-1 Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-436e10399b8so17189391cf.0 for ; Wed, 17 Apr 2024 08:13:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713366798; x=1713971598; h=mime-version:references:message-id:in-reply-to:subject:cc:to:date :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=DZg7n1wX1pk52QGJxf9KI7U4cqI2FNBfSmPHV2lo4hg=; b=wtFFNji1nMOF5KW6+DUb9/JvMip/bmD8M5CJXYkWHxGPuTLp58EzpXwVUgt5GGZFbP YPcOSXELeE7PdwQ2EsDLpZ641KT/tfTYIe8AoU15sPOtd3/+8b+Q+xczT8Uo9CvygOWS NJHiStmaArnNGAq9xmXyQsHbWi6Dln2V/SwZqcIqEzMmMI8HU33YccfqH9vsbj1XzZlu LbRcmOa1PkJrFWK/jPJg+YggjEpw6jBiobuRsd1jpaRcaLz3R+SvWWu4ay9EzsHpQUIV UM9U5cEyyM1fEgzZWxnZZFn19fG/eVa6h25tCgW3zsHf8egez49CH9tQ4Q2Sq7S8BAqi Aqag== X-Gm-Message-State: AOJu0YxnkECdpqKaxt/P7cWQeqWXrYkgFNOSDBxPKG26uPG/hIT43q5l 2cuobtrxSjNqy43WayDpm1aiV93KygTfWdmFNGl+v4Hfh/j2PYbTiPs4dPiy0cPzIJA3BSt8d2V gvGjXj5u1NQO29kn7DBZxU7V2uJcb64i+X2CvFLDEAKto2dWEMlKxpiE= X-Received: by 2002:a05:622a:28c:b0:437:884c:49b6 with SMTP id z12-20020a05622a028c00b00437884c49b6mr1181970qtw.4.1713366798254; Wed, 17 Apr 2024 08:13:18 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEjsJIYgVUyiOthAAQi436UdgHPCJ9QNFLE2NwZmFbmOwISTqOBZjdBZTeWQZ0xtse6OYWehA== X-Received: by 2002:a05:622a:28c:b0:437:884c:49b6 with SMTP id z12-20020a05622a028c00b00437884c49b6mr1181932qtw.4.1713366797904; Wed, 17 Apr 2024 08:13:17 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id i5-20020ac84885000000b00434a9af16acsm8183860qtq.6.2024.04.17.08.13.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Apr 2024 08:13:17 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Wed, 17 Apr 2024 11:13:16 -0400 (EDT) To: Nathaniel Shead cc: gcc-patches@gcc.gnu.org, Jason Merrill , Nathan Sidwell , Patrick Palka Subject: Re: [PATCH v2 1/2] c++: Standardise errors for module_may_redeclare In-Reply-To: <661cb1e4.170a0220.e8efc.335d@mx.google.com> Message-ID: References: <66014130.170a0220.d7c40.0e9b@mx.google.com> <661cb1e4.170a0220.e8efc.335d@mx.google.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SORBS_WEB,SPF_HELO_NONE,SPF_NONE,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 List-Id: On Mon, 15 Apr 2024, Nathaniel Shead wrote: > 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? LGTM > > -- >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" } > -- > 2.43.2 > >