gfortran.dg/PR82376.f90: Avoid matching a file-path.

gfortran.dg/PR82376.f90: Avoid matching a file-path.

[Hans-Peter Nilsson]

I had a file-path to sources with the substring "new" in it,
and (only) this test regressed compared to results from
another build without "new" in the name.

The test does
 ! { dg-final { scan-tree-dump-times "new" 4 "original" } }
i.e. the contents of the tree-dump-file .original needs to match
the undelimited string "new" exactly four times.  Very brittle.

In the dump-file, there are three lines with calls to new:
     D.908 = new ((integer(kind=4) *) data);
 integer(kind=4) * new (integer(kind=4) & data)
   static integer(kind=4) * new (integer(kind=4) &);

But, there's also a line, which for me and cris-elf looked like:
 _gfortran_runtime_error_at (&"At line 46 of file
  /X/xyzzynewfrob/gcc/testsuite/gfortran.dg/PR82376.f90"[1]{lb: 1 sz: 1},
  &"Pointer actual argument \'new\' is not associated"[1]{lb: 1 sz: 1});
The fourth match is obviously intended to match this line, but only
with *one* match, whereas the path can as above yield another hit.

With Tcl, the regexp for matching the " " *and* the "'"
*and* the "\" gets a bit unsightly, so I suggest just
matching the "new" calls, which according to the comment in
the test is the key point.  You can't have a file-path with
spaces and parentheses in a gcc build.  I'm also making use
of {} rather than "" needing one level of quoting; the "\("
is needed because the matched string is a regexp.

Ok to commit?

testsuite:
	* gfortran.dg/PR82376.f90: Robustify match.
---
 gcc/testsuite/gfortran.dg/PR82376.f90 | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gfortran.dg/PR82376.f90 b/gcc/testsuite/gfortran.dg/PR82376.f90
index 07143ab7e82e..b99779ce9d8a 100644
--- a/gcc/testsuite/gfortran.dg/PR82376.f90
+++ b/gcc/testsuite/gfortran.dg/PR82376.f90
@@ -2,7 +2,8 @@
 ! { dg-options "-fdump-tree-original -fcheck=pointer" }
 !
 ! Test the fix for PR82376. The pointer check was doubling up the call
-! to new. The fix reduces the count of 'new' from 5 to 4.
+! to new. The fix reduces the count of 'new' from 5 to 4, or to 3, when
+! counting only calls.
 !
 ! Contributed by José Rui Faustino de Sousa  <jrfsousa@gmail.com>
 !
@@ -56,4 +57,4 @@ contains
   end subroutine set
 
 end program main_p
-! { dg-final { scan-tree-dump-times "new" 4 "original" } }
+! { dg-final { scan-tree-dump-times { new \(} 3 "original" } }
-- 
2.11.0

Re: gfortran.dg/PR82376.f90: Avoid matching a file-path.

[Bernhard Reutner-Fischer]

On Thu, 12 Aug 2021 00:09:21 +0200
Hans-Peter Nilsson via Fortran <fortran@gcc.gnu.org> wrote:

> I had a file-path to sources with the substring "new" in it,
> and (only) this test regressed compared to results from
> another build without "new" in the name.
> 
> The test does
>  ! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> i.e. the contents of the tree-dump-file .original needs to match
> the undelimited string "new" exactly four times.  Very brittle.
> 
> In the dump-file, there are three lines with calls to new:
>      D.908 = new ((integer(kind=4) *) data);
>  integer(kind=4) * new (integer(kind=4) & data)
>    static integer(kind=4) * new (integer(kind=4) &);
> 
> But, there's also a line, which for me and cris-elf looked like:
>  _gfortran_runtime_error_at (&"At line 46 of file
>   /X/xyzzynewfrob/gcc/testsuite/gfortran.dg/PR82376.f90"[1]{lb: 1 sz: 1},
>   &"Pointer actual argument \'new\' is not associated"[1]{lb: 1 sz: 1});
> The fourth match is obviously intended to match this line, but only
> with *one* match, whereas the path can as above yield another hit.
> 
> With Tcl, the regexp for matching the " " *and* the "'"
> *and* the "\" gets a bit unsightly, so I suggest just
> matching the "new" calls, which according to the comment in
> the test is the key point.  You can't have a file-path with
> spaces and parentheses in a gcc build.  I'm also making use
> of {} rather than "" needing one level of quoting; the "\("
> is needed because the matched string is a regexp.
> 
> Ok to commit?

A wordmatch would be \mnew\M but i agree that counting calls by
{\mnew (} is fine too.

I'd call it obvious, so i dare to approve it.
OK.
thanks!
> 
> testsuite:
> 	* gfortran.dg/PR82376.f90: Robustify match.
> ---
>  gcc/testsuite/gfortran.dg/PR82376.f90 | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/testsuite/gfortran.dg/PR82376.f90 b/gcc/testsuite/gfortran.dg/PR82376.f90
> index 07143ab7e82e..b99779ce9d8a 100644
> --- a/gcc/testsuite/gfortran.dg/PR82376.f90
> +++ b/gcc/testsuite/gfortran.dg/PR82376.f90
> @@ -2,7 +2,8 @@
>  ! { dg-options "-fdump-tree-original -fcheck=pointer" }
>  !
>  ! Test the fix for PR82376. The pointer check was doubling up the call
> -! to new. The fix reduces the count of 'new' from 5 to 4.
> +! to new. The fix reduces the count of 'new' from 5 to 4, or to 3, when
> +! counting only calls.
>  !
>  ! Contributed by José Rui Faustino de Sousa  <jrfsousa@gmail.com>
>  !
> @@ -56,4 +57,4 @@ contains
>    end subroutine set
>  
>  end program main_p
> -! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> +! { dg-final { scan-tree-dump-times { new \(} 3 "original" } }

Re: gfortran.dg/PR82376.f90: Avoid matching a file-path.

[Hans-Peter Nilsson]

> From: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
> Date: Thu, 12 Aug 2021 09:03:50 +0200

> On Thu, 12 Aug 2021 00:09:21 +0200
> Hans-Peter Nilsson via Fortran <fortran@gcc.gnu.org> wrote:
> 
> > I had a file-path to sources with the substring "new" in it,
> > and (only) this test regressed compared to results from
> > another build without "new" in the name.
> > 
> > The test does
> >  ! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> > i.e. the contents of the tree-dump-file .original needs to match
> > the undelimited string "new" exactly four times.  Very brittle.
> > 
> > In the dump-file, there are three lines with calls to new:
> >      D.908 = new ((integer(kind=4) *) data);
> >  integer(kind=4) * new (integer(kind=4) & data)
> >    static integer(kind=4) * new (integer(kind=4) &);
> > 
> > But, there's also a line, which for me and cris-elf looked like:
> >  _gfortran_runtime_error_at (&"At line 46 of file
> >   /X/xyzzynewfrob/gcc/testsuite/gfortran.dg/PR82376.f90"[1]{lb: 1 sz: 1},
> >   &"Pointer actual argument \'new\' is not associated"[1]{lb: 1 sz: 1});
> > The fourth match is obviously intended to match this line, but only
> > with *one* match, whereas the path can as above yield another hit.
> > 
> > With Tcl, the regexp for matching the " " *and* the "'"
> > *and* the "\" gets a bit unsightly, so I suggest just
> > matching the "new" calls, which according to the comment in
> > the test is the key point.  You can't have a file-path with
> > spaces and parentheses in a gcc build.  I'm also making use
> > of {} rather than "" needing one level of quoting; the "\("
> > is needed because the matched string is a regexp.
> > 
> > Ok to commit?
> 
> A wordmatch would be \mnew\M but i agree that counting calls by
> {\mnew (} is fine too.

Not really; I guess I should have mentioned that I briefly
considered word-delimeters, but it'd match a subdirectory
named "new"; not to be unexpected in gcc builds.  Matching
something that can't be in a file-path (of a gcc build) is
the only way to be sure (that I can think of).

> I'd call it obvious, so i dare to approve it.
> OK.
> thanks!

Thanks, but not coming from a testsuite or fortran
maintainer I'm not sure I can actually rely on that.

OTOH, damn the torpedoes.  Committed.

> > 
> > testsuite:
> > 	* gfortran.dg/PR82376.f90: Robustify match.
> > ---
> >  gcc/testsuite/gfortran.dg/PR82376.f90 | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/gcc/testsuite/gfortran.dg/PR82376.f90 b/gcc/testsuite/gfortran.dg/PR82376.f90
> > index 07143ab7e82e..b99779ce9d8a 100644
> > --- a/gcc/testsuite/gfortran.dg/PR82376.f90
> > +++ b/gcc/testsuite/gfortran.dg/PR82376.f90
> > @@ -2,7 +2,8 @@
> >  ! { dg-options "-fdump-tree-original -fcheck=pointer" }
> >  !
> >  ! Test the fix for PR82376. The pointer check was doubling up the call
> > -! to new. The fix reduces the count of 'new' from 5 to 4.
> > +! to new. The fix reduces the count of 'new' from 5 to 4, or to 3, when
> > +! counting only calls.
> >  !
> >  ! Contributed by José Rui Faustino de Sousa  <jrfsousa@gmail.com>
> >  !
> > @@ -56,4 +57,4 @@ contains
> >    end subroutine set
> >  
> >  end program main_p
> > -! { dg-final { scan-tree-dump-times "new" 4 "original" } }
> > +! { dg-final { scan-tree-dump-times { new \(} 3 "original" } }
> 

Re: gfortran.dg/PR82376.f90: Avoid matching a file-path.

[Tobias Burnus]

On 12.08.21 14:13, Hans-Peter Nilsson via Fortran wrote:
>> I'd call it obvious, so i dare to approve it.
>> OK.
>> thanks!
> Thanks, but not coming from a testsuite or fortran
> maintainer I'm not sure I can actually rely on that.
>
> OTOH, damn the torpedoes.  Committed.

If it helps: A post-commit LGTM from my side.

I think it can also be regarded as obvious.*

Tobias

*Albeit that reminds me of a professor in mathematics who told us that
he tends to very carefully check those places where the (course,
bachelor, master, PhD, ...) student writes: 'one then trivially obtains ...'

-----------------
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