PING - OK, to commit? I have a pending patch that needs this in place.
On 05/11/2019 09:55, Mark Eggleston wrote: > > On 25/10/2019 09:03, Tobias Burnus wrote: >> Hello Mark, hi all, >> >> On 10/21/19 4:40 PM, Mark Eggleston wrote: >>> This is an extension to support a legacy feature supported by other >>> compilers such as flang and the sun compiler. As I understand it >>> this feature is associated with DEC so it enabled using >>> -fdec-char-conversions and by -fdec. >>> >>> It allows character literals to be assigned to numeric (INTEGER, >>> REAL, COMPLEX) and LOGICAL variables by direct assignment or in DATA >>> statements. >> >>> * arith.c (hollerith2representation): Use >>> OPT_Wcharacter_truncation in >>> call to gfc_warning. >> >> This has two effects: First, it permits to toggle the warning on and >> off; secondly, it disables the warning by default. It is enabled by >> -Wall, however. – I think that's acceptable: while Holleriths are >> less transparent as normal strings, for normal strings the result is >> identical. >> >> >>> + result->representation.string[result_len] = '\0'; /* For debugger */ >> >> Tiny nit: full stop after 'debugger'. > Done. >> >> >>> +/* Convert character to integer. The constant will be padded or >>> truncated. */ >> >> And here an extra space before '*/'. > Done. >> >> >>> +Allowing character literals to be used in a similar way to >>> Hollerith constants >>> +is a non-standard extension. >>> + >>> +Character literals can be used in @code{DATA} statements and >>> assignments with >> >> I wonder whether one should mention here explicitly that only >> default-kind (i.e. kind=1) character strings are permitted. >> Additionally, I wonder whether -fdec-char-conversion should be >> mentioned here – without, it is not supported and the error message >> doesn't point to this option. >> > Now mentions -fdec-char-conversion and kind=1. >> >>> + >>> + /* Flang allows character conversions similar to Hollerith >>> conversions >>> + - the first characters will be turned into ascii values. */ >> >> Is this Flang or DEC or …? I thought we talk about legacy support and >> Flang is not really legacy. >> >> > Re-worded. >>> --- a/gcc/fortran/resolve.c >>> +++ b/gcc/fortran/resolve.c >>> + if ((gfc_numeric_ts (&lhs->ts) || lhs->ts.type == BT_LOGICAL) >>> + && rhs->ts.type == BT_CHARACTER >>> + && rhs->expr_type != EXPR_CONSTANT) >>> + { >>> + gfc_error ("Cannot convert %s to %s at %L", gfc_typename (rhs), >>> + gfc_typename (lhs), &rhs->where); >>> + return false; >>> + } >> >> Maybe add a comment like: >> /* Happens with flag_dec_char_conversions for nonconstant strings. */ >> might help casual readers to understand where this if comes from. >> > Done. >> >>> @@ -331,8 +332,9 @@ gfc_conv_constant_to_tree (gfc_expr * expr) >>> gfc_build_string_const (expr->representation.length, >>> expr->representation.string)); >>> if (!integer_zerop (tmp) && !integer_onep (tmp)) >>> - gfc_warning (0, "Assigning value other than 0 or 1 to LOGICAL" >>> - " has undefined result at %L", &expr->where); >>> + gfc_warning (OPT_Wsurprising, "Assigning value other than 0 >>> or 1 " >>> + "to LOGICAL has undefined result at %L", >>> + &expr->where); >> >> I am not happy with this. We had odd issues with combining code >> generated by gfortran and ifort and Booleans types ("logical"). >> Namely, gfortran uses 0 and 1 – while ifort uses -1 and 0. When using >> ".not. var", it is sufficient to flip a single bit – either the first >> or the last bit – and it is sufficient to look only a single bit. >> >> Hence, one can get ".not. var .eqv. var". >> >> The same result one can get when assigning "-1" to logical. Hence, a >> default warning makes more sense than -Wsurprising. At least, >> -Wsurprising is enabled by default. >> >> Hence, I wonder whether your 'OPT_Wsurprising' or >> 'flag_dec_char_conversions ? OPT_Wsurprising : 0' makes more sense. >> > The latter. >> >> Actually, I don't quickly see whether 4_'string' (i.e. kind=4) >> strings are rejected or not. The gfc_character2* functions all assume >> kind=1 characters – while code like gfc_convert_constant or the >> resolve.c code only looks at BT_CHARACTER. >> On the other hand, the add_conv calls in intrintrinsic.c's >> add_conversions are only added for the default-character kind. >> >> In any case, can you add a test which checks that – even with >> -fdec-char-conversion – assigning a 2_'string' and 4_'string' to a >> integer/real/complex/logical will be rejected at compile time? >> > Did not add 2_'string' tests as 2 is not accepted as a valid kind for > characters. The addition of 4_'string' in a data statement resulted in > an ICE which has been fixed by only accepting characters of kind=1. >> Otherwise, it looks okay to me. >> >> Tobias >> >> > I noticed that warning were not produced for conversion to logicals, > re-ordering of an if..else if sequence fixes that problem. Additional > test cases have been added. > > Steve Kargl suggested a revision to the conversion warning adding > "Nonstandard" to the text this has also been done. > > Tested on x86_64 using make -j 8 check-fortran. > > Please find attached the updated patch, the change logs follow. OK to > commit? > > regards, > > Mark > > gcc/fortran/ChangeLog > > Jim MacArthur <[hidden email]> > Mark Eggleston <[hidden email]> > > * arith.c (hollerith2representation): Use > OPT_Wcharacter_truncation in > call to gfc_warning. Add character2representation, > gfc_character2int, > gfc_character2real, gfc_character2complex and gfc_character2logical. > * arith.h: Add prototypes for gfc_character2int, gfc_character2real, > gfc_character2complex and gfc_character2logical. > * expr.c (gfc_check_assign): Return true if left hand side is numeric > or logical and the right hand side is character and of kind=1. > * gfortran.texi: Add -fdec-char-conversions. > * intrinsic.c (add_conversions): Add conversions from character to > integer, real, complex and logical types for their supported kinds. > (gfc_convert_type_warn): Reorder if..else if.. sequence so that > warnings > are produced for conversion to logical. > * invoke.texi: Add option to list of options. > * invoke.texi: Add Character conversion subsection to Extensions > section. > * lang.opt: Add new option. > * options.c (set_dec_flags): Add SET_BITFLAG for > flag_dec_char_conversions. > * resolve.c (resolve_ordindary_assign): Issue error if the left hand > side is numeric or logical and the right hand side is a character > variable. > * simplify.c (gfc_convert_constant): Assign the conversion function > depending on destination type. > * trans-const.c (gfc_constant_to_tree): Use OPT_Wsurprising in > gfc_warning allowing the warning to be switched off only if > flag_dec_char_conversions is enabled. > > gcc/testsuite/gfortran.dg > > Jim MacArthur <[hidden email]> > Mark Eggleston <[hidden email]> > > PR fortran/89103 > * gfortran.dg/dec_char_conversion_in_assignment_1.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_2.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_3.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_4.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_5.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_6.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_7.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_8.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_1.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_2.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_3.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_4.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_5.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_6.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_7.f90: New test. > * gfortran.dg/hollerith5.f90: Add -Wsurprising to options. > * gfortran.dg/hollerith_legacy.f90: Add -Wsurprising to options. > * gfortran.dg/no_char_to_numeric_assign.f90: New test. > https://www.codethink.co.uk/privacy.html |
On 11/8/19 10:45 AM, Mark Eggleston wrote:
> PING - OK, to commit? I have a pending patch that needs this in place. Thanks for the ping. — Any chance, that you also work on some of the general issues once in a while (cf. Bugzilla remark below); e.g. one bug per week or fortnight? (Can be a simple one, just not to keep the number of bugs growing.) @Anyone – side question: Are there other pending patches? Ignoring one OpenMP and several OpenACC patches, I am currently aware of: * [Needs review] José's PR92142 - CFI_setpointer corrupts descriptor patch [FSF copyright assignment exists] * [Nearly ready, but needs fixes/follow-up patch] Steve's PR91178 for code like 'call foo(a, (a))' * [Approved but not committed] Paul's PR92123 patch for bind(C) + alloc scalars. Side remark: On the bind(C) side, I think PR 92189 should be fixed. Additionally, there are several recent reports on segfaults and regressions, looking Bugzilla (component = fortran, sort by change data). Back to this pending patch: On 11/5/19 10:55 AM, Mark Eggleston wrote: > I noticed that warning were not produced for conversion to logicals, > re-ordering of an if..else if sequence fixes that problem. Additional > test cases have been added. Looks as if reviewing and revisiting the patch was worthwhile :-) Thanks! > Please find attached the updated patch, the change logs follow. OK to > commit? LGTM. Thanks for patch. Cheers, Tobias PS: I still find it interesting, which Fortran code gets used; from a supercomputing centre, I heard that everything between afterwards-never-touched FORTRAN IV code to code using the latest Fortran features is used. (But most amazing I find code, which gets newly written in a FORTRAN 66 style/features with some sparse F20xx features in between – like the "publicx" example of recently [instead of 'public x' or 'public::x'; spaces aren't that expensive anymore].) > gcc/fortran/ChangeLog > > Jim MacArthur <[hidden email]> > Mark Eggleston <[hidden email]> > > * arith.c (hollerith2representation): Use > OPT_Wcharacter_truncation in > call to gfc_warning. Add character2representation, > gfc_character2int, > gfc_character2real, gfc_character2complex and gfc_character2logical. > * arith.h: Add prototypes for gfc_character2int, gfc_character2real, > gfc_character2complex and gfc_character2logical. > * expr.c (gfc_check_assign): Return true if left hand side is numeric > or logical and the right hand side is character and of kind=1. > * gfortran.texi: Add -fdec-char-conversions. > * intrinsic.c (add_conversions): Add conversions from character to > integer, real, complex and logical types for their supported kinds. > (gfc_convert_type_warn): Reorder if..else if.. sequence so that > warnings > are produced for conversion to logical. > * invoke.texi: Add option to list of options. > * invoke.texi: Add Character conversion subsection to Extensions > section. > * lang.opt: Add new option. > * options.c (set_dec_flags): Add SET_BITFLAG for > flag_dec_char_conversions. > * resolve.c (resolve_ordindary_assign): Issue error if the left hand > side is numeric or logical and the right hand side is a character > variable. > * simplify.c (gfc_convert_constant): Assign the conversion function > depending on destination type. > * trans-const.c (gfc_constant_to_tree): Use OPT_Wsurprising in > gfc_warning allowing the warning to be switched off only if > flag_dec_char_conversions is enabled. > > gcc/testsuite/gfortran.dg > > Jim MacArthur <[hidden email]> > Mark Eggleston <[hidden email]> > > PR fortran/89103 > * gfortran.dg/dec_char_conversion_in_assignment_1.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_2.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_3.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_4.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_5.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_6.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_7.f90: New test. > * gfortran.dg/dec_char_conversion_in_assignment_8.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_1.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_2.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_3.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_4.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_5.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_6.f90: New test. > * gfortran.dg/dec_char_conversion_in_data_7.f90: New test. > * gfortran.dg/hollerith5.f90: Add -Wsurprising to options. > * gfortran.dg/hollerith_legacy.f90: Add -Wsurprising to options. > * gfortran.dg/no_char_to_numeric_assign.f90: New test. |
On Fri, Nov 08, 2019 at 11:17:21AM +0100, Tobias Burnus wrote:
> Additionally, there are several recent reports on segfaults and > regressions, looking Bugzilla (component = fortran, sort by change data). > Easiest way to find open gfortran bugs is through the gfortran wiki. https://gcc.gnu.org/wiki/GFortran Go down to the "For gfrtran developers" section. -- steve |
In reply to this post by Tobias Burnus-3
On Fri, Nov 08, 2019 at 11:17:21AM +0100, Tobias Burnus wrote:
> > Jim MacArthur <[hidden email]> > > Mark Eggleston <[hidden email]> > > > > PR fortran/89103 > > * gfortran.dg/dec_char_conversion_in_assignment_1.f90: New test. > > * gfortran.dg/dec_char_conversion_in_assignment_2.f90: New test. > > * gfortran.dg/dec_char_conversion_in_assignment_3.f90: New test. > > * gfortran.dg/dec_char_conversion_in_assignment_4.f90: New test. > > * gfortran.dg/dec_char_conversion_in_assignment_5.f90: New test. > > * gfortran.dg/dec_char_conversion_in_assignment_6.f90: New test. > > * gfortran.dg/dec_char_conversion_in_assignment_7.f90: New test. > > * gfortran.dg/dec_char_conversion_in_assignment_8.f90: New test. > > * gfortran.dg/dec_char_conversion_in_data_1.f90: New test. > > * gfortran.dg/dec_char_conversion_in_data_2.f90: New test. > > * gfortran.dg/dec_char_conversion_in_data_3.f90: New test. > > * gfortran.dg/dec_char_conversion_in_data_4.f90: New test. > > * gfortran.dg/dec_char_conversion_in_data_5.f90: New test. > > * gfortran.dg/dec_char_conversion_in_data_6.f90: New test. > > * gfortran.dg/dec_char_conversion_in_data_7.f90: New test. > > * gfortran.dg/hollerith5.f90: Add -Wsurprising to options. > > * gfortran.dg/hollerith_legacy.f90: Add -Wsurprising to options. > > * gfortran.dg/no_char_to_numeric_assign.f90: New test. After full bootstrap/regtest, I've also noticed: +UNRESOLVED: gfortran.dg/dec_char_conversion_in_assignment_4.f90 -O0 compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_assignment_4.f90 -O1 compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_assignment_4.f90 -O2 compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_assignment_4.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_assignment_4.f90 -O3 -g compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_assignment_4.f90 -Os compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_data_3.f90 -O0 compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_data_3.f90 -O1 compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_data_3.f90 -O2 compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_data_3.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_data_3.f90 -O3 -g compilation failed to produce executable +UNRESOLVED: gfortran.dg/dec_char_conversion_in_data_3.f90 -Os compilation failed to produce executable Tests with dg-error are expected to fail compilation, so won't produce executable and thus can't be run. Fixed thusly, tested on x86_64-linux, committed to trunk as obvious. 2019-11-09 Jakub Jelinek <[hidden email]> * gfortran.dg/dec_char_conversion_in_assignment_4.f90: Use dg-do compile instead of dg-do run. * gfortran.dg/dec_char_conversion_in_data_3.f90: Likewise. --- testsuite/gfortran.dg/dec_char_conversion_in_assignment_4.f90 (revision 277993) +++ testsuite/gfortran.dg/dec_char_conversion_in_assignment_4.f90 (working copy) @@ -1,4 +1,4 @@ -! { dg-do run } +! { dg-do compile } ! { dg-options "-fdec -fno-dec-char-conversions" } ! ! Modified by Mark Eggleston <[hidden email]> @@ -17,4 +17,3 @@ include "dec_char_conversion_in_assignme ! { dg-error "Cannot convert" " " { target *-*-* } 47 } ! { dg-error "Cannot convert" " " { target *-*-* } 48 } ! { dg-error "Cannot convert" " " { target *-*-* } 49 } - --- testsuite/gfortran.dg/dec_char_conversion_in_data_3.f90 (revision 277993) +++ testsuite/gfortran.dg/dec_char_conversion_in_data_3.f90 (working copy) @@ -1,4 +1,4 @@ -! { dg-do run } +! { dg-do compile } ! { dg-options "-fdec -fno-dec-char-conversions" } ! ! Modified by Mark Eggleston <[hidden email]> @@ -17,4 +17,3 @@ include "dec_char_conversion_in_data_1.f ! { dg-error "Incompatible types" " " { target *-*-* } 68 } ! { dg-error "Incompatible types" " " { target *-*-* } 69 } ! { dg-error "Incompatible types" " " { target *-*-* } 70 } - Jakub |
Free forum by Nabble | Edit this page |