From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42c.google.com (mail-pf1-x42c.google.com [IPv6:2607:f8b0:4864:20::42c]) by sourceware.org (Postfix) with ESMTPS id 6349F3858D38 for ; Fri, 8 Mar 2024 23:18:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6349F3858D38 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 6349F3858D38 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42c ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709939890; cv=none; b=MGnI2EQMnY3CYKvg6tPXvxsVUJz2K1jOU+D+G48VBlXb4iyZOW2UEDEjaQFsdZ+3boqUXb1eHDg4Vs8KE+G+cQqqQi/dO4gNO0asWASOsMSLe0AkI0PyMgrod0TQTpRALMLbh4283ZDK/PUB38mWrf1u5OuncbG/MHegdFqQkik= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709939890; c=relaxed/simple; bh=C56tcArso9sMcddgqFOgCYHgNmE4Ft0gHVXoMBpmzug=; h=DKIM-Signature:Message-ID:Date:From:To:Subject:MIME-Version; b=hQDX6r2a27SkyxsO/nEcUR5ConDXOFXKCq8vW6kOOX7drG1Rmqo8XDqcLqG0YAuY+ABe3bZtOIWTJPm1avCFR6Iewky7cYMqdaHJmCua/l4+Qmj7C4mwhTFUekny8zj9GI2Z5UfRffDDZK3Fce60mjXcRO7SiFtPZH0C/XrirQw= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42c.google.com with SMTP id d2e1a72fcca58-6e5dddd3b95so1928411b3a.1 for ; Fri, 08 Mar 2024 15:18:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709939887; x=1710544687; 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=yGh/AF/xV8L6T+lgbM2h3JdT6SE1mdgsSeHrhi6LJO4=; b=Hkno/XJz4m23kaJrSMsA6Jpp59Wf4FXUOPj+ykWmZtPdQOK7IB63Ch+HMdH+BzT2gY T2sb+fDpvdqApI5zxke8owCi6Lf1BFqYQJLigr9SNI7AtBSK3dFcDiBjUS9FQFkDFeOk eSjbjuTyZlYORnnvumkHhQ4qVJUq3s/cGZxyTfmgWxY2s0GFIgwywN3YOMF/nVbpFlkz y5VqiJbrFUhYxfTbM8jmY+hKwvCM3CpUJf77mW856zfvMKnV899iHw/HvxK+U3XrVHHL CgAEjX+/sdmTegfy5uSgK7Br+MDE14WF/5oMBPN4g1w3zDYrYfVvH6wwI6orDQUtUbOm PE9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709939887; x=1710544687; 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=yGh/AF/xV8L6T+lgbM2h3JdT6SE1mdgsSeHrhi6LJO4=; b=m+BQa7xwUx7B15mvae2rXmXLQIILoKaP7ukKuZo442o1bp43h1W26IgKiwedOXzcbo 1FJJkmsv2ByHlluwnokBNFqDV2qP1f5xhFUd3X8/JbxYpeOLBSgFzhRiLgyibNfpezAZ 1Fiya8l8d6vRxQY5z1s5cH8QX1oKkn1ED6MXxHJO0Y53d7QqPIsXqrP57Vh73qZriZB1 SyVSh97QefibZb2Nm4HLHOaAeTZCLVswfCZmxy9jTZ405cPIvof9ctAHimQr0JnROcNV ACyZw1Hdf+356N9zm+d6Fsu7emvXl1eGjZGR+MzP11t388SS/Gu2J6oOj6zngNHRlZ3b FINQ== X-Gm-Message-State: AOJu0YyDpcXMIGc+6VtFHutzyHohlYT7N0UCTEqw1lznsudZ22xqT1AA 442I5GH5JpWK/h9TzCCylGj5ns9uTa8fyHDDmp5L6X/wl4tcCsbF X-Google-Smtp-Source: AGHT+IEkzVPlRgQHijWrNY3fFwAUUBokQjGTO3bOl008ebYQR5ajTcREyR85KPhOBPPYGjihkkY2KQ== X-Received: by 2002:a05:6a20:438b:b0:1a1:e41:3edb with SMTP id i11-20020a056a20438b00b001a10e413edbmr217693pzl.11.1709939886968; Fri, 08 Mar 2024 15:18:06 -0800 (PST) Received: from Thaum. (202-161-100-107.tpgi.com.au. [202.161.100.107]) by smtp.gmail.com with ESMTPSA id v11-20020aa799cb000000b006e56ff4f0bdsm216976pfi.29.2024.03.08.15.18.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Mar 2024 15:18:06 -0800 (PST) Message-ID: <65eb9cae.a70a0220.5c928.117d@mx.google.com> X-Google-Original-Message-ID: Date: Sat, 9 Mar 2024 10:18:01 +1100 From: Nathaniel Shead To: Jason Merrill Cc: gcc-patches@gcc.gnu.org, Nathan Sidwell Subject: Re: [PATCH v2] c++: Check module attachment instead of just purview when necessary [PR112631] References: <655b2b3f.a70a0220.ca3c4.d564@mx.google.com> <65642242.170a0220.8bb8e.28e1@mx.google.com> <65ea7e1e.630a0220.d1c27.476d@mx.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE,WEIRD_PORT 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 Fri, Mar 08, 2024 at 10:19:52AM -0500, Jason Merrill wrote: > On 3/7/24 21:55, Nathaniel Shead wrote: > > On Mon, Nov 27, 2023 at 03:59:39PM +1100, Nathaniel Shead wrote: > > > On Thu, Nov 23, 2023 at 03:03:37PM -0500, Nathan Sidwell wrote: > > > > On 11/20/23 04:47, Nathaniel Shead wrote: > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write > > > > > access. > > > > > > > > > > -- >8 -- > > > > > > > > > > Block-scope declarations of functions or extern values are not allowed > > > > > when attached to a named module. Similarly, class member functions are > > > > > not inline if attached to a named module. However, in both these cases > > > > > we currently only check if the declaration is within the module purview; > > > > > it is possible for such a declaration to occur within the module purview > > > > > but not be attached to a named module (e.g. in an 'extern "C++"' block). > > > > > This patch makes the required adjustments. > > > > > > > > > > > > Ah I'd been puzzling over the default inlinedness of member-fns of > > > > block-scope structs. Could you augment the testcase to make sure that's > > > > right too? > > > > > > > > Something like: > > > > > > > > // dg-module-do link > > > > export module Mod; > > > > > > > > export auto Get () { > > > > struct X { void Fn () {} }; > > > > return X(); > > > > } > > > > > > > > > > > > /// > > > > import Mod > > > > void Frob () { Get().Fn(); } > > > > > > > > > > I gave this a try and it indeed doesn't work correctly; 'Fn' needs to be > > > marked 'inline' for this to link (whether or not 'Get' itself is > > > inline). I've tried tracing the code to work out what's going on but > > > I've been struggling to work out how all the different flags (e.g. > > > TREE_PUBLIC, TREE_EXTERNAL, TREE_COMDAT, DECL_NOT_REALLY_EXTERN) > > > interact, which flags we want to be set where, and where the decision of > > > what function definitions to emit is actually made. > > > > > > I did find that doing 'mark_used(decl)' on all member functions in > > > block-scope structs seems to work however, but I wonder if that's maybe > > > too aggressive or if there's something else we should be doing? > > > > I got around to looking at this again, here's an updated version of this > > patch. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > (I'm not sure if 'start_preparsed_function' is the right place to be > > putting this kind of logic or if it should instead be going in > > 'grokfndecl', e.g. decl.cc:10761 where the rules for making local > > functions have no linkage are initially determined, but I found this > > easier to implement: happy to move around though if preferred.) > > > > -- >8 -- > > > > Block-scope declarations of functions or extern values are not allowed > > when attached to a named module. Similarly, class member functions are > > not inline if attached to a named module. However, in both these cases > > we currently only check if the declaration is within the module purview; > > it is possible for such a declaration to occur within the module purview > > but not be attached to a named module (e.g. in an 'extern "C++"' block). > > This patch makes the required adjustments. > > > > While implementing this we discovered that block-scope local functions > > are not correctly emitted, causing link failures; this patch also > > corrects some assumptions here and ensures that they are emitted when > > needed. > > > > PR c++/112631 > > > > gcc/cp/ChangeLog: > > > > * cp-tree.h (named_module_attach_p): New function. > > * decl.cc (start_decl): Check for attachment not purview. > > (grokmethod): Likewise. > > These changes are OK; the others I want to consider more. > Thanks, I can commit this as a separate commit if you prefer? > > +export auto n_n() { > > + internal(); > > + struct X { void f() { internal(); } }; > > + return X{}; > > Hmm, is this not a prohibited exposure? Seems like X has no linkage because > it's at block scope, and the deduced return type names it. > > I know we try to support this "voldemort" pattern, but is that actually > correct? > > Jason > I had similar doubts, but this is not an especially uncommon pattern in the wild either. I also asked some other people for their thoughts and got told: "no linkage" doesn't mean it's ill-formed to name it in other scopes. It means a declaration in another scope cannot correspond to it And that the wording in [basic.link] p2.4 is imprecise. (Apparently they were going to raise a core issue about this too, I think?) As for whether it's an exposure, looking at [basic.link] p15, the entity 'X' doesn't actually appear to be TU-local: it doesn't have a name with internal linkage (no linkage is different) and is not declared or introduced within the definition of a TU-local entity (n_n is not TU-local). So I think this example is OK, but this example is not: namespace { auto x() { struct X { void f() {} }; return X{}; } } export auto illegal() { return x(); } Which we correctly complain about already: error: 'struct {anonymous}::x()::X' references internal linkage entity 'auto {anonymous}::x()' 6 | struct X { void f() {} }; | ^ Nathaniel