From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 80096 invoked by alias); 23 Mar 2015 14:58:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 79687 invoked by uid 89); 23 Mar 2015 14:58:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: smtp21.services.sfr.fr Received: from smtp21.services.sfr.fr (HELO smtp21.services.sfr.fr) (93.17.128.1) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 23 Mar 2015 14:58:33 +0000 Received: from filter.sfr.fr (localhost [86.72.15.254]) by msfrf2109.sfr.fr (SMTP Server) with ESMTP id BF5157000148; Mon, 23 Mar 2015 15:58:29 +0100 (CET) Authentication-Results: sfrmc.priv.atos.fr; dkim=none (no signature); dkim-adsp=none (no policy) header.from=mikael.morin@sfr.fr Received: from tolstoi.localhost (254.15.72.86.rev.sfr.net [86.72.15.254]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by msfrf2109.sfr.fr (SMTP Server) with ESMTP id 167247000092; Mon, 23 Mar 2015 15:58:18 +0100 (CET) X-SFR-UUID: 20150323145820920.167247000092@msfrf2109.sfr.fr Message-ID: <551029F7.4080601@sfr.fr> Date: Mon, 23 Mar 2015 14:58:00 -0000 From: Mikael Morin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Andre Vehreschild CC: GCC-Fortran-ML , GCC-Patches-ML , Antony Lewis Subject: Re: [Patch 1/2, Fortran, pr60322] [OOP] Incorrect bounds on polymorphic dummy array References: <20150226181717.480e282c@vepi2> <551006FF.1080704@sfr.fr> <20150323134357.6af740d1@vepi2> In-Reply-To: <20150323134357.6af740d1@vepi2> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg01182.txt.bz2 Le 23/03/2015 13:43, Andre Vehreschild a écrit : > Hi Mikael, > > thanks for looking at the patch. Please note, that Paul has sent an addendum to > the patches for 60322, which I deliberately have attached. > >> 26/02/2015 18:17, Andre Vehreschild a écrit : >>> This first patch is only preparatory and does not change any of the >>> semantics of gfortran at all. >> Sure? > > With the counterexample you found below, this of course is a wrong statement. > >>> diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c >>> index ab6f7a5..d28cf77 100644 >>> --- a/gcc/fortran/expr.c >>> +++ b/gcc/fortran/expr.c >>> @@ -4059,10 +4060,10 @@ gfc_lval_expr_from_sym (gfc_symbol *sym) >>> lval->symtree = gfc_find_symtree (sym->ns->sym_root, sym->name); >>> >>> /* It will always be a full array. */ >>> - lval->rank = sym->as ? sym->as->rank : 0; >>> + as = sym->as; >>> + lval->rank = as ? as->rank : 0; >>> if (lval->rank) >>> - gfc_add_full_array_ref (lval, sym->ts.type == BT_CLASS ? >>> - CLASS_DATA (sym)->as : sym->as); >>> + gfc_add_full_array_ref (lval, as); >> >> This is a change of semantics. Or do you know that sym->ts.type != >> BT_CLASS? > > You are completely right. I have made a mistake here. I have to tell the truth, > I never ran a regtest with only part 1 of the patches applied. The second part > of the patch will correct this, by setting the variable as depending on whether > type == BT_CLASS or not. Sorry for the mistake. > >>> diff --git a/gcc/fortran/trans-decl.c b/gcc/fortran/trans-decl.c >>> index 3664824..e571a17 100644 >>> --- a/gcc/fortran/trans-decl.c >>> +++ b/gcc/fortran/trans-decl.c >>> @@ -1013,16 +1017,24 @@ gfc_build_dummy_array_decl (gfc_symbol * sym, tree >>> dummy) tree decl; >>> tree type; >>> gfc_array_spec *as; >>> + symbol_attribute *array_attr; >>> char *name; >>> gfc_packed packed; >>> int n; >>> bool known_size; >>> >>> - if (sym->attr.pointer || sym->attr.allocatable >>> - || (sym->as && sym->as->type == AS_ASSUMED_RANK)) >>> + /* Use the array as and attr. */ >>> + as = sym->as; >>> + array_attr = &sym->attr; >>> + >>> + /* The pointer attribute is always set on a _data component, therefore >>> check >>> + the sym's attribute only. */ >>> + if (sym->attr.pointer || array_attr->allocatable >>> + || (as && as->type == AS_ASSUMED_RANK)) >>> return dummy; >>> >> Any reason to sometimes use array_attr, sometimes not, like here? >> By the way, the comment is misleading: for classes, there is the >> class_pointer attribute (and it is a pain, I know). > > Yes, and a good one. Array_attr is sometimes sym->attr and sometimes > CLASS_DATA(sym)->attr aka sym->ts.u.derived->components->attr. In the later > case .pointer is always set to 1 in the _data component's attr. I.e., the above > if, would always yield true for a class_array, which is not intended, but rather > destructive. I know about the class_pointer attribute, but I figured, that it > is not relevant here. Any idea how to formulate the comment better, to reflect > what I just explained? > This pointer stuff is very difficult to swallow to me. I understand that for classes, the CLASS_DATA (sym)->pointer is always set, but almost everywhere the checks for pointerness are like (sym->ts.type != BT_CLASS && sym->attr.pointer) || (sym->ts.type == BT_CLASS && CLASS_DATA (sym)->attr.class_pointer) and I don't see a convincing reason to have it different here. At least gfc_is_nodesc_array should return 0 if sym->ts.type == BT_CLASS which solves the problem there; for the other cases, I think that class_pointer should be looked at. gfc_build_class_symbol clears the sym->attr.pointer flag for class containers so it doesn't make sense to test that flag. Mikael