From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ej1-x634.google.com (mail-ej1-x634.google.com [IPv6:2a00:1450:4864:20::634]) by sourceware.org (Postfix) with ESMTPS id EACA43858CD1; Thu, 29 Feb 2024 02:49:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EACA43858CD1 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 EACA43858CD1 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::634 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709174948; cv=none; b=D7TNvQRpjyQ39d+8zOCcspK9HSlj2LRScpgJcypK0anzK1x2OgQHtlLrIZLjoz70Zpu9Hqm3KfAk78gw/Gv8XtCROLUbC0ho1LvWR0nzJFJkyn+/JqyAyQuEbgxH2G0u6hY4k6wkyiBVR3vvUqz6jLzlfSGFVJvn31Bgv3dtYY8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1709174948; c=relaxed/simple; bh=sWY2gdtGybqT4mJVEiMuQOcn7d73Noee/0Y8Pfa6GVU=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=m6TXLlAezcPpTalaIqxZblRbXO6Gg6vhw49lKt62snTGJzBisE80Pw9XJAwQsiv2TAsfiJRxATLFxUCN+GFz2tXBxu/PqEPjJxcX9DV1aUD/W3e6LQvX67/Naq8N6btjhuh1H2xI3nXD3S1tAMb8PIicW8b742kvOoV3yIgzHAs= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ej1-x634.google.com with SMTP id a640c23a62f3a-a3f829cde6dso70241066b.0; Wed, 28 Feb 2024 18:49:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709174939; x=1709779739; darn=gcc.gnu.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=YuPPs0hXVrUl5OUpV75iF37vQueivthAUU4jBddtKe0=; b=htnhUOxJwQz6aROYpdrJkYbW1puFDW+alb3OUSiYhUgBm+SIEKXi9xYzIIFmrJFk2t fl5MUIC/MaANghG0ZgSFptSaUDEm0r528eXQIXo6rk0sRDWbjNHaChRP1+3kUt4MX3ku S5aFZYX6OeBtnAV5OijJ5KT4vEvEqcx15dkDI9wAIHtWG5bAQMtRijp1XI1qe3REnKTe ma4fVWJK5S8X00UETPGNAn1i6bvyE9lvnlEzyXYMM3xPvo8UEmuckCx4gYdX64JG60FG pZ4J06YvgkfraRRrY6kOgDSFwOafEuGVOcU02UbHHadqU781x/kTbuONz1kpjqF4vpNY Bngw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709174939; x=1709779739; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YuPPs0hXVrUl5OUpV75iF37vQueivthAUU4jBddtKe0=; b=NaMtFsl4WSohM2EEtho/ct6+8+lKcPlNFPizoog8XvZclKvEEXI4CINQwISaY3quDl qp+l7ZYW8k6tWZ6v7qmZbZW6k2Oe+HyHQodUer1YkVuEx/EipVJgZhuPoDSQkSDIRo9n XxUQNNQy8/iGaF54jkQVfTNnbpnXaD2GeTjuOxLsGSc5BPg4aquv21aMZVSbUWPGdKm7 cTEmUKt3A/rboGx4csVVuxWsrgy72MdZlpwXzxmkc1d06Z2RPcqG8I7/pZbk0yADpPjx Cmp44Sn2DfeOvcgTA3ZoPYZHIJ/EK6DaOpkRJSawvVLoPeUXcNDoyRXMZZuT+oVeVY4x rL6Q== X-Forwarded-Encrypted: i=1; AJvYcCVTQxs7X9bRGjeZGkHV5Reu/s/sF5dFhx28vgEjJoHMVOijUUQPkOU5Sr/t1DQmHdzg7Vam475Nn4rY0KkKWIBg+PIs X-Gm-Message-State: AOJu0YyvhNmBbz92byYsdH6rHbjR3lDlzVaJKoKVRwGija/70PMyPgj8 c8jr2TGBZemXnBB4SeB7UpTOubHvSmhERXbemCTyGRGG4WzIEGyQwebNS5W5y1en3YPZJFAUuTI Rxw2W7Hfz0MdrcTsEe36tmP0wzbM= X-Google-Smtp-Source: AGHT+IGVf/SptOGNeYJM9+F/PBsXDRlHIDcXlQrqtaPT4QK5HJlcyTxhTZ5fVWkiZUsdNuIHw+Jt8FSh6LjQ8PKoDoQ= X-Received: by 2002:a17:906:d8b4:b0:a3e:b263:d769 with SMTP id qc20-20020a170906d8b400b00a3eb263d769mr455870ejb.4.1709174939167; Wed, 28 Feb 2024 18:48:59 -0800 (PST) MIME-Version: 1.0 References: <5a4694d0-c610-4297-8255-77a15cda92c4@gmx.de> In-Reply-To: From: Alexander Westbrooks Date: Wed, 28 Feb 2024 20:48:47 -0600 Message-ID: Subject: Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268] To: Harald Anlauf Cc: gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: Hello, I meant to add a link to the commit to the previous email: https://gcc.gnu.org/git/gitweb.cgi?p=3Dgcc.git;h=3Dedfe198084338691d0facc86= bf8dfa6ede3ca676 Thanks, Alexander Westbrooks On Wed, Feb 28, 2024 at 8:24=E2=80=AFPM Alexander Westbrooks wrote: > > Hello, > > I've updated the patch with those changes, ran through the gcc-verify > step and fixed up the commit, and then pushed it to the trunk. > > Thank you for your feedback, and I look forward to working on GFortran. > > Thanks, > > Alexander Westbrooks > > On Wed, Feb 28, 2024 at 1:55=E2=80=AFPM Harald Anlauf wro= te: > > > > Hi Alex, > > > > this is now mostly correct, with the following exceptions: > > > > First, you should notice that the formatting of the commit message, > > when checked using "git gcc-verify", needs minor corrections. You > > will be guided how to fix this yourself. > > > > Second, testcase pdt_37.f03 has an undeclared dummy argument, which > > can be detected by adding "implicit none" (I usually use that > > whenever implicit typing is not wanted explicitly). I would get: > > > > pdt_37.f03:33:47: > > > > 33 | subroutine assumed_len_param_ptr(this, that) > > | 1 > > Error: Symbol 'that' at (1) has no IMPLICIT type; did you mean 'this'? > > > > I assume you want to uncomment the declaration of dummy 'that'. > > > > Third, I still see a - minor - indentation/tabbing/space issue here: > > > > diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc > > index 44f89f6afb4..852e0820e6a 100644 > > --- a/gcc/fortran/resolve.cc > > +++ b/gcc/fortran/resolve.cc > > [...] > > + if ( resolve_bindings_derived->attr.pdt_template > > + && gfc_pdt_is_instance_of (resolve_bindings_derived, > > + CLASS_DATA (me_arg)->ts.u.derived) > > + && (me_arg->param_list !=3D NULL) > > + && (gfc_spec_list_type (me_arg->param_list, > > + CLASS_DATA(me_arg)->ts.u.derived) > > + !=3D SPEC_ASSUMED)) > > > > OK with the above fixed. > > > > Thanks for the patch! > > > > Harald > > > > On 2/28/24 07:24, Alexander Westbrooks wrote: > > > Harald, > > > > > > Jerry helped me figure out my editor settings so that I could fix > > > whitespace and formatting issues in my code. With my editor configure= d > > > correctly, I saw that my code was not conforming to coding standards > > > as I previously thought it was. I have fixed those things and updated > > > my patch. Thank you for your patience. > > > > > > Let me know if this is okay to push to the trunk. > > > > > > Thanks, > > > > > > Alexander Westbrooks > > > > > > On Sun, Feb 25, 2024 at 2:40=E2=80=AFPM Alexander Westbrooks > > > wrote: > > >> > > >> Harald, > > >> > > >> Thank you for reviewing my code. I've been doing research and debugg= ing to investigate the error thrown by Intel and NAG for the deferred param= eter in the dummy variable declaration. I found where the problem was and a= dded the fix as part of my patch. I've attached the patch as a file, which = also includes your feedback and suggested fixes. I've updated the test case= pdt_37.f03 to check for the POINTER or ALLOCATABLE error as you suggested. > > >> > > >> All regression tests pass, including the new ones, after including t= he fix for the POINTER or ALLOCATABLE error for CLASS declarations of PDTs = when deferred length parameters are used. This was tested on WSL 2, with Ub= untu 20.04 distro. > > >> > > >> Is this okay to push to the trunk? > > >> > > >> Thanks, > > >> > > >> Alexander Westbrooks > > >> > > >> > > >> On Sun, Feb 11, 2024 at 2:11=E2=80=AFPM Harald Anlauf wrote: > > >>> > > >>> Hi Alex, > > >>> > > >>> I've been unable to apply your patch to my local trunk, likely due = to > > >>> whitespace issues my newsreader handles differently from your site. > > >>> I see it inline instead of attached. > > >>> > > >>> A few general remarks: > > >>> > > >>> Please follow the general recommendation regarding style if possibl= e, > > >>> see https://www.gnu.org/prep/standards/standards.html#Formatting > > >>> regarding formatting/whitespace use (5.1) and comments (5.2) > > >>> > > >>> Also, when an error message text spans multiple lines, please place= the > > >>> whitespace at the end of a line, not at the beginning of the new on= e: > > >>> > > >>>> + if ( resolve_bindings_derived->attr.pdt_template && > > >>>> + !gfc_pdt_is_instance_of(resolve_bindings_derived, > > >>>> + CLASS_DATA(me_arg)->ts.u.derived)) > > >>>> + { > > >>>> + gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be= of" > > >>>> + " the parametric derived-type %qs", me_arg->name, proc->n= ame, > > >>> > > >>> gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be= of " > > >>> "the parametric derived-type %qs", me_arg->name, > > >>> proc->name, > > >>> > > >>>> + me_arg->name, &where, resolve_bindings_derived->name); > > >>>> + goto error; > > >>>> + } > > >>> > > >>> The following change is almost unreadable: the lnegthy comment is s= plit > > >>> over three parts and almost hides the code. Couldn't this be combi= ned > > >>> into one comment before the function? > > >>> > > >>>> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc > > >>>> index fddf68f8398..11f4bac0415 100644 > > >>>> --- a/gcc/fortran/symbol.cc > > >>>> +++ b/gcc/fortran/symbol.cc > > >>>> @@ -5172,6 +5172,35 @@ gfc_type_is_extension_of (gfc_symbol *t1, g= fc_symbol > > >>>> *t2) > > >>>> return gfc_compare_derived_types (t1, t2); > > >>>> } > > >>>> > > >>>> +/* Check if a parameterized derived type t2 is an instance of a P= DT > > >>>> template t1 */ > > >>>> + > > >>>> +bool > > >>>> +gfc_pdt_is_instance_of(gfc_symbol *t1, gfc_symbol *t2) > > >>>> +{ > > >>>> + if ( !t1->attr.pdt_template || !t2->attr.pdt_type ) > > >>>> + return false; > > >>>> + > > >>>> + /* > > >>>> + in decl.cc, gfc_get_pdt_instance, a pdt instance is given a 3 > > >>>> character prefix "Pdt", followed > > >>>> + by an underscore list of the kind parameters, up to a maximum= of 8. > > >>>> + > > >>>> + So to check if a PDT Type corresponds to the template, extrac= t the > > >>>> core derive_type name, > > >>>> + and then see if it is type compatible by name... > > >>>> + > > >>>> + For example: > > >>>> + > > >>>> + Pdtf_2_2 -> extract out the 'f' -> see if the derived type 'f= ' is > > >>>> compatible with symbol t1 > > >>>> + */ > > >>>> + > > >>>> + // Starting at index 3 of the string in order to skip past the = 'Pdt' > > >>>> prefix > > >>>> + // Also, here the length of the template name is used in order = to avoid > > >>>> the > > >>>> + // kind parameter suffixes that are placed at the end of PDT in= stance > > >>>> names. > > >>>> + if ( !(strncmp(&(t2->name[3]), t1->name, strlen(t1->name)) =3D= =3D 0) ) > > >>>> + return false; > > >>>> + > > >>>> + return true; > > >>>> +} > > >>>> + > > >>>> > > >>>> /* Check if two typespecs are type compatible (F03:5.1.1.2): > > >>>> If ts1 is nonpolymorphic, ts2 must be the same type. > > >>> > > >>> The following testcase tests for errors. I tried Intel and NAG on = it > > >>> after commenting the 'contains' section of the type desclaration. > > >>> Both complained about subroutine deferred_len_param, e.g. > > >>> > > >>> Intel: > > >>> A colon may only be used as a type parameter value in the declarati= on of > > >>> an object that has the POINTER or ALLOCATABLE attribute. [THIS] > > >>> class(param_deriv_type(:)), intent(inout) :: this > > >>> > > >>> NAG: > > >>> Entity THIS of type PARAM_DERIV_TYPE(A=3D:) has a deferred length t= ype > > >>> parameter but is not a data pointer or allocatable > > >>> > > >>> Do we detect this after your patch? If the answer is yes, > > >>> can we add another subroutine where we check for this error? > > >>> (the dg-error suggests we only expect assumed len type parameters.) > > >>> If no, maybe add a comment in the testcase that this subroutine > > >>> may need updating later. > > >>> > > >>>> diff --git a/gcc/testsuite/gfortran.dg/pdt_37.f03 > > >>>> b/gcc/testsuite/gfortran.dg/pdt_37.f03 > > >>>> new file mode 100644 > > >>>> index 00000000000..68d376fad25 > > >>>> --- /dev/null > > >>>> +++ b/gcc/testsuite/gfortran.dg/pdt_37.f03 > > >>>> @@ -0,0 +1,34 @@ > > >>>> +! { dg-do compile } > > >>>> +! > > >>>> +! Tests the fixes for PR82943. > > >>>> +! > > >>>> +! This test focuses on the errors produced by incorrect LEN param= eters for > > >>>> dummy > > >>>> +! arguments of PDT Typebound Procedures. > > >>>> +! > > >>>> +! Contributed by Alexander Westbrooks > > >>>> +! > > >>>> +module test_len_param > > >>>> + > > >>>> + type :: param_deriv_type(a) > > >>>> + integer, len :: a > > >>>> + contains > > >>>> + procedure :: assumed_len_param ! Good. No error = expected. > > >>>> + procedure :: deferred_len_param ! { dg-error "All= LEN type > > >>>> parameters of the passed dummy argument" } > > >>>> + procedure :: fixed_len_param ! { dg-error "All= LEN type > > >>>> parameters of the passed dummy argument" } > > >>>> + end type > > >>>> + > > >>>> +contains > > >>>> + subroutine assumed_len_param(this) > > >>>> + class(param_deriv_type(*)), intent(inout) :: this > > >>>> + end subroutine > > >>>> + > > >>>> + subroutine deferred_len_param(this) > > >>>> + class(param_deriv_type(:)), intent(inout) :: this > > >>>> + end subroutine > > >>>> + > > >>>> + subroutine fixed_len_param(this) > > >>>> + class(param_deriv_type(10)), intent(inout) :: this > > >>>> + end subroutine > > >>>> + > > >>>> +end module > > >>>> + > > >>> > >