Re: Request to improve docs: Fortran integer ranges vs. machine representation

2024-01-28 Thread Elias Toivanen
Guys, it's unproductive to discuss GFortran internals or the nuances of the 
standard with an end-user like me ;-)

AFAIK, the implementation is compliant and correct. My angle here was 
completely different. The question is whether this aspect of Fortran leads to 
confusion and subsequently to duplicate bug reports and other such noise, which 
is away from your precious time. If so, the way the error messages are worded 
and the way your docs are written play a crucial role in mitigating the issue.

Enabling the `-pedantic` flag in myself, in the listing (primary.cc:286 
) 

```
if (gfc_range_check (e) != ARITH_OK)
{
  gfc_error ("Integer too big for its kind at %C. This check can be "
 "disabled with the option %<-fno-range-check%>");

  gfc_free_expr (e);
  return MATCH_ERROR;
}
```

the if-block will be entered for any of the current and future error codes, 
while the error message only talks about the magnitude of the input.

If on the other hand you deem that working on this is not worth the effort, the 
issue is naturally resolved. 


--
Best regards / Ystävällisesti
Elias Toivanen
elias.a.toiva...@gmail.com


> On 27. Jan 2024, at 0.38, FX Coudert  wrote:
> 
>> Interesting example.
>> 
>> % gfcx -o z a.f90 &&  ./z
>> -128
>> % gfcx -o z -pedantic a.f90 && ./z
>> a.f90:5:20:
>> 
>>   5 |   data j /-128_int8/
>> |1
>> Error: Integer too big for its kind at (1). This check can be disabled with 
>> the option ‘-fno-range-check’
> 
> That qualifies as a compiler bug, I think. Our documentation for -pedantic 
> states: “Issue warnings for uses of extensions to Fortran.” and "Valid 
> Fortran programs should compile properly with or without this option.”
> 
> The same is true of the following, which is also valid Fortran since 95 :
> 
> use iso_fortran_env
> implicit none
> complex, parameter :: z = (-128_int8, -128_int8)
> print *, z
> end
> 
> Right now it fails to compile with -pedantic.
> 
> Or are they illegal because of how the range should be be symmetric? I can’t 
> quite find the language in the standard for that, actually. To me, they’re 
> valid signed-int-literal-constant.
> 
> FX
> 
> 
> PS: I’m going to ignore the cases of the P and DT edit descriptors, because 
> they’re not allowed to have a kind value, and therefore the corner cases 
> occur for values too big to be actually relevant to anything.




Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]

2024-01-28 Thread Mikael Morin

Le 24/01/2024 à 22:39, Harald Anlauf a écrit :

Dear all,

this patch is actually only a followup fix to generate the proper name
of an array reference in derived-type components for the runtime error
message generated for the bounds-checking code.  Without the proper
part ref, not only a user may get confused: I was, too...

The testcase is compile-only, as it is only important to check the
strings used in the error messages.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


Hello,

the change proper looks good, and is an improvement.  But I'm a little 
concerned by the production of references like in the test x1%vv%z which 
could be confusing and is strictly speaking invalid fortran (multiple 
non-scalar components).  Did you consider generating x1%vv(?,?)%zz or 
x1%vv(...)%z or similar?


There is another nit about the test, which has dg-output and 
dg-shouldfail despite being only a compile-time test.


Otherwise looks good.

Mikael


Re: [PATCH] Fortran: NULL actual to optional dummy with VALUE attribute [PR113377]

2024-01-28 Thread Mikael Morin

Le 25/01/2024 à 22:26, Harald Anlauf a écrit :

Dear all,

this is the third patch in a series that addresses dummy arguments
with the VALUE attribute, now handling the passing of NULL actual
arguments.  It is based on the refactoring in the previous patch
and reuses the handling of absent arguments.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


OK, thanks.


Fwd: [PATCH] testsuite, gfortan: Update link flags [PR112862].

2024-01-28 Thread Toon Moene





 Forwarded Message 
Subject: [PATCH] testsuite, gfortan: Update link flags [PR112862].
Date: Sun, 28 Jan 2024 15:03:08 +
From: Iain Sandoe 
Reply-To: i...@sandoe.co.uk
To: gcc-patc...@gcc.gnu.org
CC: r...@cebitec.uni-bielefeld.de

Tested on i686, x86_64, aarch64 Darwin, x86_64, aarch64 Linux,
OK for trunk?
thanks, Iain

--- 8< ---

The regressions here are caused by two issues:
1. In some cases there is no generated runpath for libatomic
2. In other cases there are duplicate paths.

This patch simplifies the addition of the options in the main
gfortran exp and removes the duplicates elewhere.

We need to add options to locate libgfortran and the dependent libs
libquadmath (supporting REAL*16) and libatomic (supporting operations
used by coarrays).  Usually '-L' options are added to point to the
relevant directories for the uninstalled libraries.

In cases where libraries are available as both shared and convenience
some additional checks are made.

For some targets -static- options are handled by specs substitution
and need a '-B' option rather than '-L'.  For Darwin, when embedded
runpaths are in use (the default for all versions after macOS 10.11),
'-B' is also needed to provide the runpath.

When '-B' is used, this results in a '-L' for each path that exists (so
that appending a '-L' as well is a needless duplicate).  There are also
cases where tools warn for duplicates, leading to spurious fails.

PR target/112862

gcc/testsuite/ChangeLog:

* gfortran.dg/coarray/caf.exp: Remove duplicate additions of
libatomic handling.
* gfortran.dg/dg.exp: Likewise.
* lib/gfortran.exp: Decide on whether to present -B or -L to
reference the paths to uninstalled libgfortran, libqadmath and
libatomic and use that to generate the link flags.
---
 gcc/testsuite/gfortran.dg/coarray/caf.exp | 16 +
 gcc/testsuite/gfortran.dg/dg.exp  | 20 ---
 gcc/testsuite/lib/gfortran.exp| 73 +++
 3 files changed, 51 insertions(+), 58 deletions(-)

diff --git a/gcc/testsuite/gfortran.dg/coarray/caf.exp 
b/gcc/testsuite/gfortran.dg/coarray/caf.exp

index dae46bd92fa..31c13cd34e5 100644
--- a/gcc/testsuite/gfortran.dg/coarray/caf.exp
+++ b/gcc/testsuite/gfortran.dg/coarray/caf.exp
@@ -70,18 +70,6 @@ proc dg-compile-aux-modules { args } {
 }
 }
 -# Add -latomic only where supported.  Assume built-in support elsewhere.
-set maybe_atomic_lib ""
-if [check_effective_target_libatomic_available] {
-if ![is_remote host] {
-   if [info exists TOOL_OPTIONS] {
-	set maybe_atomic_lib "[atomic_link_flags [get_multilibs 
${TOOL_OPTIONS}]]"

-   } else {
-   set maybe_atomic_lib "[atomic_link_flags [get_multilibs]]"
-   }
-}
-}
-
 # Main loop.
 foreach test [lsort [glob -nocomplain 
$srcdir/$subdir/*.\[fF\]{,90,95,03,08} ]] {
 # If we're only testing specific files and this isn't one of them, 
skip it.
@@ -105,14 +93,14 @@ foreach test [lsort [glob -nocomplain 
$srcdir/$subdir/*.\[fF\]{,90,95,03,08} ]]

 foreach flags $option_list {
verbose "Testing $nshort (single), $flags" 1
 set gfortran_aux_module_flags "-fcoarray=single $flags"
-   dg-test $test "-fcoarray=single $flags" $maybe_atomic_lib
+   dg-test $test "-fcoarray=single $flags" {}
cleanup-modules ""
 }
  foreach flags $option_list {
verbose "Testing $nshort (libcaf_single), $flags" 1
 set gfortran_aux_module_flags "-fcoarray=lib $flags -lcaf_single"
-   dg-test $test "-fcoarray=lib $flags -lcaf_single" $maybe_atomic_lib
+   dg-test $test "-fcoarray=lib $flags -lcaf_single" {}
cleanup-modules ""
 }
 }
diff --git a/gcc/testsuite/gfortran.dg/dg.exp 
b/gcc/testsuite/gfortran.dg/dg.exp

index f936fd38644..7a9cb89c194 100644
--- a/gcc/testsuite/gfortran.dg/dg.exp
+++ b/gcc/testsuite/gfortran.dg/dg.exp
@@ -54,27 +54,7 @@ proc dg-compile-aux-modules { args } {
 }
 }
 -# coarray tests might need libatomic.  Assume that it is either not 
needed or

-# provided by builtins if it's not available.
-set maybe_atomic_lib ""
-if [check_effective_target_libatomic_available] {
-if ![is_remote host] {
-   if [info exists TOOL_OPTIONS] {
-	set maybe_atomic_lib "[atomic_link_flags [get_multilibs 
${TOOL_OPTIONS}]]"

-   } else {
-   set maybe_atomic_lib "[atomic_link_flags [get_multilibs]]"
-   }
-} else {
-set maybe_atomic_lib ""
-}
-}
-
 set all_flags $DEFAULT_FFLAGS
-if { $maybe_atomic_lib != "" } {
-   foreach f $maybe_atomic_lib {
- lappend all_flags $f
-   }
-}
  # Main loop.
 gfortran-dg-runtest [lsort \
diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp
index c3e258b410b..1ccb81ccec5 100644
--- a/gcc/testsuite/lib/gfortran.exp
+++ b/gcc/testsuite/lib/gfortran.exp
@@ -79,6 +79,7 @@ proc gfortran_link_flags { paths } {
 global ld_library_path
 global GFORTRAN_UNDER_TEST
 glob

Re: [PATCH] testsuite, gfortan: Update link flags [PR112862].

2024-01-28 Thread FX Coudert
> Tested on i686, x86_64, aarch64 Darwin, x86_64, aarch64 Linux,
> OK for trunk?

Looks good to me. Please leave 48h before pushing for other Fortran maintainers 
to comment if they see something I missed (in particular the coarrays part).

FX

Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]

2024-01-28 Thread Harald Anlauf

Hi Mikael,

Am 28.01.24 um 12:39 schrieb Mikael Morin:

Le 24/01/2024 à 22:39, Harald Anlauf a écrit :

Dear all,

this patch is actually only a followup fix to generate the proper name
of an array reference in derived-type components for the runtime error
message generated for the bounds-checking code.  Without the proper
part ref, not only a user may get confused: I was, too...

The testcase is compile-only, as it is only important to check the
strings used in the error messages.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald


Hello,

the change proper looks good, and is an improvement.  But I'm a little
concerned by the production of references like in the test x1%vv%z which
could be confusing and is strictly speaking invalid fortran (multiple
non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
x1%vv(...)%z or similar?


yes, that seems very reasonable, given that this is what NAG does.

We also have spurious %_data in some error messages that I'll try
to get rid off.


There is another nit about the test, which has dg-output and
dg-shouldfail despite being only a compile-time test.


I'll remove that.  With gcc-13 the runtime check would trigger
in the wrong line but the failure of the check is dealt with
by another testcase in gcc-14.


Otherwise looks good.

Mikael


I'll come up with a revised patch.  Thanks for the feedback so far!

Harald




Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]

2024-01-28 Thread Steve Kargl
On Sun, Jan 28, 2024 at 08:56:24PM +0100, Harald Anlauf wrote:
> 
> Am 28.01.24 um 12:39 schrieb Mikael Morin:
> > Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
> > > Dear all,
> > > 
> > > this patch is actually only a followup fix to generate the proper name
> > > of an array reference in derived-type components for the runtime error
> > > message generated for the bounds-checking code.  Without the proper
> > > part ref, not only a user may get confused: I was, too...
> > > 
> > > The testcase is compile-only, as it is only important to check the
> > > strings used in the error messages.
> > > 
> > > Regtested on x86_64-pc-linux-gnu.  OK for mainline?
> > > 
> > 
> > the change proper looks good, and is an improvement.  But I'm a little
> > concerned by the production of references like in the test x1%vv%z which
> > could be confusing and is strictly speaking invalid fortran (multiple
> > non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
> > x1%vv(...)%z or similar?
> 
> yes, that seems very reasonable, given that this is what NAG does.
> 
> We also have spurious %_data in some error messages that I'll try
> to get rid off.
> 

I haven't looked at the patch, but sometimes (if not always) things
like _data are marked with attr.artificial.  You might see if this
will help with suppressing spurious messages.



-- 
Steve


Re: [PATCH] Fortran: use name of array component in runtime error message [PR30802]

2024-01-28 Thread rep . dot . nop
On 28 January 2024 22:43:37 CET, Steve Kargl 
 wrote:
>On Sun, Jan 28, 2024 at 08:56:24PM +0100, Harald Anlauf wrote:
>> 
>> Am 28.01.24 um 12:39 schrieb Mikael Morin:
>> > Le 24/01/2024 à 22:39, Harald Anlauf a écrit :
>> > > Dear all,
>> > > 
>> > > this patch is actually only a followup fix to generate the proper name
>> > > of an array reference in derived-type components for the runtime error
>> > > message generated for the bounds-checking code.  Without the proper
>> > > part ref, not only a user may get confused: I was, too...
>> > > 
>> > > The testcase is compile-only, as it is only important to check the
>> > > strings used in the error messages.
>> > > 
>> > > Regtested on x86_64-pc-linux-gnu.  OK for mainline?
>> > > 
>> > 
>> > the change proper looks good, and is an improvement.  But I'm a little
>> > concerned by the production of references like in the test x1%vv%z which
>> > could be confusing and is strictly speaking invalid fortran (multiple
>> > non-scalar components).  Did you consider generating x1%vv(?,?)%zz or
>> > x1%vv(...)%z or similar?
>> 
>> yes, that seems very reasonable, given that this is what NAG does.
>> 
>> We also have spurious %_data in some error messages that I'll try
>> to get rid off.
>> 
>
>I haven't looked at the patch, but sometimes (if not always) things
>like _data are marked with attr.artificial.  You might see if this
>will help with suppressing spurious messages.
>

Reminds me of
https://inbox.sourceware.org/fortran/2024231748.376086cd@nbbrfq/

Maybe thats missing, i did not apply that yet, did i?

HTH