From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ciao.gmane.io (ciao.gmane.io [116.202.254.214]) by sourceware.org (Postfix) with ESMTPS id 9AC423858C50 for ; Mon, 28 Mar 2022 20:03:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9AC423858C50 Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1nYvak-0008HX-Mj for fortran@gcc.gnu.org; Mon, 28 Mar 2022 22:03:54 +0200 X-Injected-Via-Gmane: http://gmane.org/ To: fortran@gcc.gnu.org From: Harald Anlauf Subject: Re: [PATCH] PR fortran/50549 - should detect different type parameters in structure constructors Date: Mon, 28 Mar 2022 22:03:46 +0200 Message-ID: References: <5a7fa9f6-7155-9dc6-d847-e44ccccb447a@codesourcery.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------ib3AKV1dFuRMDAWGug8IpipW" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Content-Language: en-US In-Reply-To: <5a7fa9f6-7155-9dc6-d847-e44ccccb447a@codesourcery.com> Cc: gcc-patches@gcc.gnu.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, BODY_8BITS, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, KAM_SHORT, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: fortran@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Fortran mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Mar 2022 20:03:58 -0000 Message-ID: <20220328200346.ZY8oqiV5kuqXKrg9K3VA03MEmUe6Ao5ecFFihkW0cqo@z> This is a multi-part message in MIME format. --------------ib3AKV1dFuRMDAWGug8IpipW Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi Tobias, Am 28.03.22 um 12:05 schrieb Tobias Burnus: > Thanks for the patch! LGTM and I think GCC 12 is still okay. > > However, I have a nit: > >> --- a/gcc/fortran/resolve.cc >> +++ b/gcc/fortran/resolve.cc >> @@ -1375,11 +1375,22 @@ resolve_structure_cons (gfc_expr *expr, int init) >> ... >> +           long len_a, len_b; >> +           len_a = mpz_get_si (comp->ts.u.cl->length->value.integer); >> +           len_b = mpz_get_si >> (cons->expr->ts.u.cl->length->value.integer); >> +           gfc_error ("Unequal character lengths (%ld/%ld) for pointer " >> +                      "component %qs in constructor at %L", >> +                      len_a, len_b, comp->name, &cons->expr->where); > > 'long' might be int32_t instead of int64_t (e.g. on Windows, I think both > MinGW32 and MinGW64, but I am not quite sure). Thus, I wonder whether it > makes more sense to use: > >   HOST_WIDE_INT, gfc_mpz_get_hwi() and '%wd' > > I note that '%wd' (and '%lld') is only supported since last August > (commit https://gcc.gnu.org/r12-3044-g1b507b1e3c5 ), but now that it is, > I think we should use it at places where the value can be larger than > INT_MAX. using HOST_WIDE_INT as in the updated patch (sort of) works, but for some reason I do not yet understand the format check kicks in for gfc_error, producing: ../../gcc-trunk/gcc/fortran/resolve.cc: In function 'bool resolve_structure_cons(gfc_expr*, int)': ../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: unknown conversion type character 'w' in format [-Wformat=] la, lb, comp->name, &cons->expr->where); ^ ../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: unknown conversion type character 'w' in format [-Wformat=] ../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'long int' [-Wformat=] ../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: format '%L' expects argument of type 'locus*', but argument 3 has type 'long int' [-Wformat=] ../../gcc-trunk/gcc/fortran/resolve.cc:1388:43: warning: too many arguments for format [-Wformat-extra-args] This would likely lead to a bootstrap error. Do I need to add some forgotten include? Or some annotation to suppress the warning? Or should I rather convert the character lengths via sprintf first before generating the error message? (That would be the quick fix.) > I think at some point, we should also check the rest of the code and > change those mpz_get_si to gfc_mpz_get_hwi which can exceed INT_MAX. > Likewise, some of the %ld/%lu or %lld/%llu code should be also converted > to %wd/%wu. > > Tobias > > PS: For using HWI with 'sprintf' instead of diagnostic's error/warning, > HOST_WIDE_INT_PRINT_DEC exists and has to be used. All current cases of printing a HOST_WIDE_INT in gcc/fortran/ use 'sprintf', and I did not find any other use of %wd/%wu. So the mentioned implementation is not really stressed yet... ;-) Thanks, Harald > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, > 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: > Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; > Registergericht München, HRB 106955 > --------------ib3AKV1dFuRMDAWGug8IpipW Content-Type: text/x-patch; charset=UTF-8; name="0001-Fortran-character-length-of-pointer-assignments-in-s.patch" Content-Disposition: attachment; filename*0="0001-Fortran-character-length-of-pointer-assignments-in-s.pa"; filename*1="tch" Content-Transfer-Encoding: base64 RnJvbSA3ZWZkMDYxMzI2MWM1ZDIxMjBlMTg5Mzg3ZjRiOTE2OTE3YzI1NjgzIE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBIYXJhbGQgQW5sYXVmIDxhbmxhdWZAZ214LmRlPgpE YXRlOiBTdW4sIDI3IE1hciAyMDIyIDIxOjM1OjE1ICswMjAwClN1YmplY3Q6IFtQQVRDSF0g Rm9ydHJhbjogY2hhcmFjdGVyIGxlbmd0aCBvZiBwb2ludGVyIGFzc2lnbm1lbnRzIGluIHN0 cnVjdHVyZQogY29uc3RydWN0b3JzCgpnY2MvZm9ydHJhbi9DaGFuZ2VMb2c6CgoJUFIgZm9y dHJhbi81MDU0OQoJKiByZXNvbHZlLmNjIChyZXNvbHZlX3N0cnVjdHVyZV9jb25zKTogUmVq ZWN0IHBvaW50ZXIgYXNzaWdubWVudHMKCW9mIGNoYXJhY3RlciB3aXRoIGRpZmZlcmVudCBs ZW5ndGhzIGluIHN0cnVjdHVyZSBjb25zdHJ1Y3Rvci4KCmdjYy90ZXN0c3VpdGUvQ2hhbmdl TG9nOgoKCVBSIGZvcnRyYW4vNTA1NDkKCSogZ2ZvcnRyYW4uZGcvY2hhcl9wb2ludGVyX2Fz c2lnbl83LmY5MDogTmV3IHRlc3QuCi0tLQogZ2NjL2ZvcnRyYW4vcmVzb2x2ZS5jYyAgICAg ICAgICAgICAgICAgICAgICAgIHwgMTMgKysrKysrLQogLi4uL2dmb3J0cmFuLmRnL2NoYXJf cG9pbnRlcl9hc3NpZ25fNy5mOTAgICAgIHwgMzggKysrKysrKysrKysrKysrKysrKwogMiBm aWxlcyBjaGFuZ2VkLCA1MCBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCiBjcmVhdGUg bW9kZSAxMDA2NDQgZ2NjL3Rlc3RzdWl0ZS9nZm9ydHJhbi5kZy9jaGFyX3BvaW50ZXJfYXNz aWduXzcuZjkwCgpkaWZmIC0tZ2l0IGEvZ2NjL2ZvcnRyYW4vcmVzb2x2ZS5jYyBiL2djYy9m b3J0cmFuL3Jlc29sdmUuY2MKaW5kZXggNTUyMmJlNzUxOTkuLjI5MDc2NzcyM2Q4IDEwMDY0 NAotLS0gYS9nY2MvZm9ydHJhbi9yZXNvbHZlLmNjCisrKyBiL2djYy9mb3J0cmFuL3Jlc29s dmUuY2MKQEAgLTEzNzUsMTEgKzEzNzUsMjIgQEAgcmVzb2x2ZV9zdHJ1Y3R1cmVfY29ucyAo Z2ZjX2V4cHIgKmV4cHIsIGludCBpbml0KQogCSAgJiYgY29tcC0+dHMudS5jbC0+bGVuZ3Ro LT5leHByX3R5cGUgPT0gRVhQUl9DT05TVEFOVAogCSAgJiYgY29ucy0+ZXhwci0+dHMudS5j bCAmJiBjb25zLT5leHByLT50cy51LmNsLT5sZW5ndGgKIAkgICYmIGNvbnMtPmV4cHItPnRz LnUuY2wtPmxlbmd0aC0+ZXhwcl90eXBlID09IEVYUFJfQ09OU1RBTlQKLQkgICYmIGNvbnMt PmV4cHItPnJhbmsgIT0gMAogCSAgJiYgbXB6X2NtcCAoY29ucy0+ZXhwci0+dHMudS5jbC0+ bGVuZ3RoLT52YWx1ZS5pbnRlZ2VyLAogCQkgICAgICBjb21wLT50cy51LmNsLT5sZW5ndGgt PnZhbHVlLmludGVnZXIpICE9IDApCiAJeworCSAgaWYgKGNvbXAtPmF0dHIucG9pbnRlcikK KwkgICAgeworCSAgICAgIEhPU1RfV0lERV9JTlQgbGEsIGxiOworCSAgICAgIGxhID0gZ2Zj X21wel9nZXRfaHdpIChjb21wLT50cy51LmNsLT5sZW5ndGgtPnZhbHVlLmludGVnZXIpOwor CSAgICAgIGxiID0gZ2ZjX21wel9nZXRfaHdpIChjb25zLT5leHByLT50cy51LmNsLT5sZW5n dGgtPnZhbHVlLmludGVnZXIpOworCSAgICAgIGdmY19lcnJvciAoIlVuZXF1YWwgY2hhcmFj dGVyIGxlbmd0aHMgKCV3ZC8ld2QpIGZvciBwb2ludGVyICIKKwkJCSAiY29tcG9uZW50ICVx cyBpbiBjb25zdHJ1Y3RvciBhdCAlTCIsCisJCQkgbGEsIGxiLCBjb21wLT5uYW1lLCAmY29u cy0+ZXhwci0+d2hlcmUpOworCSAgICAgIHQgPSBmYWxzZTsKKwkgICAgfQorCiAJICBpZiAo Y29ucy0+ZXhwci0+ZXhwcl90eXBlID09IEVYUFJfVkFSSUFCTEUKKwkgICAgICAmJiBjb25z LT5leHByLT5yYW5rICE9IDAKIAkgICAgICAmJiBjb25zLT5leHByLT5zeW10cmVlLT5uLnN5 bS0+YXR0ci5mbGF2b3IgPT0gRkxfUEFSQU1FVEVSKQogCSAgICB7CiAJICAgICAgLyogV3Jh cCB0aGUgcGFyYW1ldGVyIGluIGFuIGFycmF5IGNvbnN0cnVjdG9yIChFWFBSX0FSUkFZKQpk aWZmIC0tZ2l0IGEvZ2NjL3Rlc3RzdWl0ZS9nZm9ydHJhbi5kZy9jaGFyX3BvaW50ZXJfYXNz aWduXzcuZjkwIGIvZ2NjL3Rlc3RzdWl0ZS9nZm9ydHJhbi5kZy9jaGFyX3BvaW50ZXJfYXNz aWduXzcuZjkwCm5ldyBmaWxlIG1vZGUgMTAwNjQ0CmluZGV4IDAwMDAwMDAwMDAwLi4wOGJk ZjE3NmQ4YgotLS0gL2Rldi9udWxsCisrKyBiL2djYy90ZXN0c3VpdGUvZ2ZvcnRyYW4uZGcv Y2hhcl9wb2ludGVyX2Fzc2lnbl83LmY5MApAQCAtMCwwICsxLDM4IEBACishIHsgZGctZG8g Y29tcGlsZSB9CishIFBSIGZvcnRyYW4vNTA1NDkgLSBzaG91bGQgcmVqZWN0IHBvaW50ZXIg YXNzaWdubWVudHMgb2YgZGlmZmVyZW50IGxlbmd0aHMKKyEgaW4gc3RydWN0dXJlIGNvbnN0 cnVjdG9ycworCitwcm9ncmFtIHRlc3QKKyAgaW1wbGljaXQgbm9uZQorICB0eXBlIHQKKyAg ICAgY2hhcmFjdGVyKDIpLCBwb2ludGVyIDo6ICBwMgorICBlbmQgdHlwZSB0CisgIHR5cGUg dDIKKyAgICAgY2hhcmFjdGVyKDIpLCBwb2ludGVyIDo6ICBwKDopCisgIGVuZCB0eXBlIHQy CisgIHR5cGUgdGQKKyAgICAgY2hhcmFjdGVyKDopLCBwb2ludGVyIDo6ICBwZAorICBlbmQg dHlwZSB0ZAorICBpbnRlcmZhY2UKKyAgICAgZnVuY3Rpb24gZjEgKCkKKyAgICAgICBjaGFy YWN0ZXIoMSksIHBvaW50ZXIgOjogZjEKKyAgICAgZW5kIGZ1bmN0aW9uIGYxCisgICAgIGZ1 bmN0aW9uIGYyICgpCisgICAgICAgY2hhcmFjdGVyKDIpLCBwb2ludGVyIDo6IGYyCisgICAg IGVuZCBmdW5jdGlvbiBmMgorICBlbmQgaW50ZXJmYWNlCisKKyAgY2hhcmFjdGVyKDEpLCAg ICB0YXJnZXQgIDo6ICBwMQorICBjaGFyYWN0ZXIoMSksICAgIHBvaW50ZXIgOjogIHExKDop CisgIGNoYXJhY3RlcigyKSwgICAgcG9pbnRlciA6OiAgcTIoOikKKyAgdHlwZSh0KSAgOjog dQorICB0eXBlKHQyKSA6OiB1MgorICB0eXBlKHRkKSA6OiB2CisgIHUgID0gdChwMSkgICAg ISB7IGRnLWVycm9yICJVbmVxdWFsIGNoYXJhY3RlciBsZW5ndGhzIiB9CisgIHUgID0gdChm MSgpKSAgISB7IGRnLWVycm9yICJVbmVxdWFsIGNoYXJhY3RlciBsZW5ndGhzIiB9CisgIHUg ID0gdChmMigpKSAgISBPSworICB1MiA9IHQyKHExKSAgICEgeyBkZy1lcnJvciAiVW5lcXVh bCBjaGFyYWN0ZXIgbGVuZ3RocyIgfQorICB1MiA9IHQyKHEyKSAgICEgT0sKKyAgdiAgPSB0 ZChwMSkgICAhIE9LCisgIHYgID0gdGQoZjEoKSkgISBPSworZW5kCi0tIAoyLjM0LjEKCg== --------------ib3AKV1dFuRMDAWGug8IpipW--