From patchwork Mon Nov 7 17:42:42 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 124153 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id CF7ADB6F7D for ; Tue, 8 Nov 2011 04:43:10 +1100 (EST) Received: (qmail 4548 invoked by alias); 7 Nov 2011 17:43:09 -0000 Received: (qmail 4539 invoked by uid 22791); 7 Nov 2011 17:43:07 -0000 X-SWARE-Spam-Status: No, hits=-7.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 07 Nov 2011 17:42:47 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pA7HgijG009816 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 7 Nov 2011 12:42:45 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id pA7HgiSP004701; Mon, 7 Nov 2011 12:42:44 -0500 Received: from [0.0.0.0] (ovpn-113-127.phx2.redhat.com [10.3.113.127]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id pA7Hgg4O000362; Mon, 7 Nov 2011 12:42:43 -0500 Message-ID: <4EB81892.4030502@redhat.com> Date: Mon, 07 Nov 2011 12:42:42 -0500 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20111001 Thunderbird/7.0.1 MIME-Version: 1.0 To: gcc-patches List CC: Vincenzo Innocente Subject: Re: C++/c-common PATCH for c++/35688 (wrong visibility of template instantiation) References: <4EB75F14.3060400@redhat.com> In-Reply-To: <4EB75F14.3060400@redhat.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On 11/06/2011 11:31 PM, Jason Merrill wrote: > The function constrain_visibility_for_template tries to set the > visibility of a template instantiation properly by giving it the minimum > visibility of the template itself and the template arguments. But this > PR points out that we were failing to do that in the case that the > template is within a namespace with a visibility attribute, because then > it gets DECL_VISIBILITY_SPECIFIED. This patch fixes that by re-using > some of the C front end's visibility code, so that we can further > constrain visibility on a decl with DECL_VISIBILITY_SPECIFIED so long as > it doesn't actually have a visibility attribute. Vincenzo pointed out that this still didn't fix the visibility of class template instantiations, and on further reflection I think we want lesser template argument visibility to override even an explicit attribute on the template. That's also what the documentation says. So I've reverted the previous approach and now just give template argument visibility higher priority. Tested x86_64-pc-linux-gnu, applying to trunk. commit 145941ce3947108c9572379a33b3f48e5c539146 Author: Jason Merrill Date: Mon Nov 7 12:16:11 2011 -0500 PR c++/35688 * decl2.c (constrain_visibility): Return void. Add tmpl parm which gives the constraint priority over an attribute. (constrain_visibility_for_template, determine_visibility): Adjust. * pt.c (instantiate_class_template_1): Call determine_visibility. diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index 80fb0c3..17be3ad 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -1954,10 +1954,12 @@ type_visibility (tree type) } /* Limit the visibility of DECL to VISIBILITY, if not explicitly - specified (or if VISIBILITY is static). */ + specified (or if VISIBILITY is static). If TMPL is true, this + constraint is for a template argument, and takes precedence + over explicitly-specified visibility on the template. */ -static bool -constrain_visibility (tree decl, int visibility) +static void +constrain_visibility (tree decl, int visibility, bool tmpl) { if (visibility == VISIBILITY_ANON) { @@ -1974,16 +1976,11 @@ constrain_visibility (tree decl, int visibility) DECL_NOT_REALLY_EXTERN (decl) = 1; } } - /* We check decl_has_visibility_attr rather than - DECL_VISIBILITY_SPECIFIED here because we want other considerations - to override visibility from a namespace or #pragma. */ else if (visibility > DECL_VISIBILITY (decl) - && !decl_has_visibility_attr (decl)) + && (tmpl || !DECL_VISIBILITY_SPECIFIED (decl))) { DECL_VISIBILITY (decl) = (enum symbol_visibility) visibility; - return true; } - return false; } /* Constrain the visibility of DECL based on the visibility of its template @@ -2019,7 +2016,7 @@ constrain_visibility_for_template (tree decl, tree targs) } } if (vis) - constrain_visibility (decl, vis); + constrain_visibility (decl, vis, true); } } @@ -2132,7 +2129,7 @@ determine_visibility (tree decl) if (underlying_vis == VISIBILITY_ANON || (CLASS_TYPE_P (underlying_type) && CLASSTYPE_VISIBILITY_SPECIFIED (underlying_type))) - constrain_visibility (decl, underlying_vis); + constrain_visibility (decl, underlying_vis, false); else DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT; } @@ -2140,7 +2137,7 @@ determine_visibility (tree decl) { /* tinfo visibility is based on the type it's for. */ constrain_visibility - (decl, type_visibility (TREE_TYPE (DECL_NAME (decl)))); + (decl, type_visibility (TREE_TYPE (DECL_NAME (decl))), false); /* Give the target a chance to override the visibility associated with DECL. */ @@ -2207,14 +2204,14 @@ determine_visibility (tree decl) if (decl_anon_ns_mem_p (decl)) /* Names in an anonymous namespace get internal linkage. This might change once we implement export. */ - constrain_visibility (decl, VISIBILITY_ANON); + constrain_visibility (decl, VISIBILITY_ANON, false); else if (TREE_CODE (decl) != TYPE_DECL) { /* Propagate anonymity from type to decl. */ int tvis = type_visibility (TREE_TYPE (decl)); if (tvis == VISIBILITY_ANON || ! DECL_VISIBILITY_SPECIFIED (decl)) - constrain_visibility (decl, tvis); + constrain_visibility (decl, tvis, false); } else if (no_linkage_check (TREE_TYPE (decl), /*relaxed_p=*/true)) /* DR 757: A type without linkage shall not be used as the type of a @@ -2225,7 +2222,7 @@ determine_visibility (tree decl) Since non-extern "C" decls need to be defined in the same translation unit, we can make the type internal. */ - constrain_visibility (decl, VISIBILITY_ANON); + constrain_visibility (decl, VISIBILITY_ANON, false); /* If visibility changed and DECL already has DECL_RTL, ensure symbol flags are updated. */ diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 493e3e6..52f4d47 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -8600,6 +8600,8 @@ instantiate_class_template_1 (tree type) { CLASSTYPE_VISIBILITY_SPECIFIED (type) = 1; CLASSTYPE_VISIBILITY (type) = CLASSTYPE_VISIBILITY (pattern); + /* Adjust visibility for template arguments. */ + determine_visibility (TYPE_MAIN_DECL (type)); } CLASSTYPE_FINAL (type) = CLASSTYPE_FINAL (pattern); diff --git a/gcc/testsuite/g++.dg/ext/visibility/template8.C b/gcc/testsuite/g++.dg/ext/visibility/template8.C new file mode 100644 index 0000000..e491882 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/visibility/template8.C @@ -0,0 +1,26 @@ +// PR c++/35688 +// { dg-require-visibility "" } +// { dg-options "-fvisibility=hidden" } + +// { dg-final { scan-hidden "_Z1gI1BEvT_" } } +// { dg-final { scan-hidden "_Z1gI1AI1BEEvT_" } } + +// Test that template argument visibility takes priority even over an +// explicit visibility attribute on a template. + +template +struct __attribute ((visibility ("default"))) A { }; +template +void g(T) __attribute ((visibility ("default"))); + +struct B { }; + +template +void g(T) +{ } + +int main() +{ + g(B()); + g(A()); +}