Skip to contentSkip to conversation

Improve and test Fortran xfuncname regex

PATCH In review Philippe Blain via GitGitGadget 4 participants 7 messages Aug 12, 2020 – Aug 13, 2020

lore
Share & tools
lore

Philippe Blain via GitGitGadget

author
#0 lore raw

cover Improve and test Fortran xfuncname regex

This series adds tests for the Fortran xfuncname regex and fixes a shortcoming of the regex.

Philippe Blain (2):
userdiff: add tests for Fortran xfuncname regex
userdiff: improve Fortran xfuncname regex

 t/t4018/fortran-block-data          |  5 +++++
 t/t4018/fortran-comment             | 13 +++++++++++++
 t/t4018/fortran-comment-keyword     | 14 ++++++++++++++
 t/t4018/fortran-comment-legacy      | 13 +++++++++++++
 t/t4018/fortran-comment-legacy-star | 13 +++++++++++++
 t/t4018/fortran-external-function   |  9 +++++++++
 t/t4018/fortran-external-subroutine |  5 +++++
 t/t4018/fortran-module              |  5 +++++
 t/t4018/fortran-module-procedure    | 13 +++++++++++++
 t/t4018/fortran-program             |  5 +++++
 userdiff.c                          |  6 +++++-
 11 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/fortran-block-data
 create mode 100644 t/t4018/fortran-comment
 create mode 100644 t/t4018/fortran-comment-keyword
 create mode 100644 t/t4018/fortran-comment-legacy
 create mode 100644 t/t4018/fortran-comment-legacy-star
 create mode 100644 t/t4018/fortran-external-function
 create mode 100644 t/t4018/fortran-external-subroutine
 create mode 100644 t/t4018/fortran-module
 create mode 100644 t/t4018/fortran-module-procedure
 create mode 100644 t/t4018/fortran-program

base-commit: 4f0a8be78499454eac3985b6e7e144b8376ab0a5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-700%2Fphil-blain%2Ffortran-better-userdiff-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-700/phil-blain/fortran-better-userdiff-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/700

Signature · 1 line
gitgitgadget
author
#1 lore raw

PATCH 1/2 userdiff: add tests for Fortran xfuncname regex

From: Philippe Blain <[email protected]>

The Fortran userdiff patterns, introduced in 909a5494f8 (userdiff.c: add builtin fortran regex patterns, 2010-09-10), predate the test infrastructure for xfuncname patterns, introduced in bfa7d01413 (t4018:
an infrastructure to test hunk headers, 2014-03-21).

Add tests for the Fortran xfuncname patterns. The test 't/t4018/fortran-comment-keyword' documents a shortcoming of the regex that is fixed in a subsequent commit.

While at it, add descriptive comments for the different parts of the regex.

Signed-off-by Philippe Blain <[email protected]>
---
 t/t4018/fortran-block-data          |  5 +++++
 t/t4018/fortran-comment             | 13 +++++++++++++
 t/t4018/fortran-comment-keyword     | 15 +++++++++++++++
 t/t4018/fortran-comment-legacy      | 13 +++++++++++++
 t/t4018/fortran-comment-legacy-star | 13 +++++++++++++
 t/t4018/fortran-external-function   |  9 +++++++++
 t/t4018/fortran-external-subroutine |  5 +++++
 t/t4018/fortran-module              |  5 +++++
 t/t4018/fortran-module-procedure    | 13 +++++++++++++
 t/t4018/fortran-program             |  5 +++++
 userdiff.c                          |  4 ++++
 11 files changed, 100 insertions(+)
 create mode 100644 t/t4018/fortran-block-data
 create mode 100644 t/t4018/fortran-comment
 create mode 100644 t/t4018/fortran-comment-keyword
 create mode 100644 t/t4018/fortran-comment-legacy
 create mode 100644 t/t4018/fortran-comment-legacy-star
 create mode 100644 t/t4018/fortran-external-function
 create mode 100644 t/t4018/fortran-external-subroutine
 create mode 100644 t/t4018/fortran-module
 create mode 100644 t/t4018/fortran-module-procedure
 create mode 100644 t/t4018/fortran-program
t/t4018/fortran-block-data new file +5 -0
new file mode 100644
index 0000000000..63d4e21d0a
@@ -0,0 +1,5 @@
+       BLOCK DATA RIGHT
+       
+       COMMON /B/ C, ChangeMe
+       DATA C, ChangeMe  / 2.0, 6.0 / 
+       END
t/t4018/fortran-comment new file +13 -0
new file mode 100644
index 0000000000..7b10d17658
@@ -0,0 +1,13 @@
+      module a
+
+      contains
+
+      ! subroutine wrong
+      subroutine RIGHT
+      ! subroutine wrong
+
+      real ChangeMe
+
+      end subroutine RIGHT
+
+      end module a
t/t4018/fortran-comment-keyword new file +15 -0
new file mode 100644
index 0000000000..c5dbdb4c61
@@ -0,0 +1,15 @@
+      module a
+
+      contains
+
+      subroutine RIGHT (funcA, funcB)
+
+      real funcA  ! grid function a
+      real funcB  ! grid function b
+
+      real ChangeMe
+      integer broken
+
+      end subroutine RIGHT
+
+      end module a
t/t4018/fortran-comment-legacy new file +13 -0
new file mode 100644
index 0000000000..53cd062c1e
@@ -0,0 +1,13 @@
+      module a
+
+      contains
+
+C subroutine wrong
+      subroutine RIGHT
+C subroutine wrong
+
+      real ChangeMe
+
+      end subroutine RIGHT
+
+      end module a
t/t4018/fortran-comment-legacy-star new file +13 -0
new file mode 100644
index 0000000000..2cbcdc3d8a
@@ -0,0 +1,13 @@
+      module a
+
+      contains
+
+* subroutine wrong
+      subroutine RIGHT
+* subroutine wrong
+
+      real ChangeMe
+
+      end subroutine RIGHT
+
+      end module a
t/t4018/fortran-external-function new file +9 -0
new file mode 100644
index 0000000000..5a2d85d3aa
@@ -0,0 +1,9 @@
+function RIGHT(a, b) result(c)
+
+integer, intent(in) :: ChangeMe
+integer, intent(in) :: b
+integer, intent(out) :: c
+
+c = a+b
+
+end function RIGHT
t/t4018/fortran-external-subroutine new file +5 -0
new file mode 100644
index 0000000000..4ce85fea13
@@ -0,0 +1,5 @@
+subroutine RIGHT
+
+real ChangeMe
+
+end subroutine RIGHT
t/t4018/fortran-module new file +5 -0
new file mode 100644
index 0000000000..c4b737dac3
@@ -0,0 +1,5 @@
+module RIGHT
+
+use ChangeMe
+
+end module RIGHT
t/t4018/fortran-module-procedure new file +13 -0
new file mode 100644
index 0000000000..1ce6d854c2
@@ -0,0 +1,13 @@
+ module RIGHT
+
+   implicit none
+   private
+
+   interface letters  ! generic interface
+      module procedure aaaa, &
+                       bbbb, &
+                       ChangeMe, &
+                       dddd
+   end interface
+   
+end module RIGHT
t/t4018/fortran-program new file +5 -0
new file mode 100644
index 0000000000..4616895e4b
@@ -0,0 +1,5 @@
+program RIGHT
+
+call ChangeMe
+
+end program RIGHT
userdiff.c +4 -0
index 1df884ef0b..707d82435a 100644
@@ -46,9 +46,13 @@ PATTERNS("elixir",
 	 /* Not real operators, but should be grouped */
 	 "|:?%[A-Za-z0-9_.]\\{\\}?"),
 IPATTERN("fortran",
+	 /* Don't match comment lines */
 	 "!^([C*]|[ \t]*!)\n"
+	 /* Don't match 'module procedure' lines */
 	 "!^[ \t]*MODULE[ \t]+PROCEDURE[ \t]\n"
+	 /* Program, module, block data */
 	 "^[ \t]*((END[ \t]+)?(PROGRAM|MODULE|BLOCK[ \t]+DATA"
+		/* Subroutines and functions */
 		"|([^'\" \t]+[ \t]+)*(SUBROUTINE|FUNCTION))[ \t]+[A-Z].*)$",
 	 /* -- */
 	 "[a-zA-Z][a-zA-Z0-9_]*"
Signature · 1 line
gitgitgadget
author
#2 lore raw

PATCH 2/2 userdiff: improve Fortran xfuncname regex

In reply to Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>

The third part of the Fortran xfuncname regex wants to match the beginning of a subroutine or function, so it allows for all characters except ', " or whitespace before the keyword 'function' or 'subroutine'. This is meant to match the 'recursive', 'elemental' or 'pure' keywords, as well as function return types, and to prevent matches inside strings.

However, the negated set does not contain the ! comment character, so a line with an end-of-line comment containing the keyword 'function' or 'subroutine' followed by another word is mistakenly chosen as a hunk header.

Improve the regex by adding ! to the negated set.

Signed-off-by Philippe Blain <[email protected]>
---
 t/t4018/fortran-comment-keyword | 1 -
 userdiff.c                      | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
t/t4018/fortran-comment-keyword +0 -1
index c5dbdb4c61..e9206a5379 100644
@@ -8,7 +8,6 @@
       real funcB  ! grid function b
 
       real ChangeMe
-      integer broken
 
       end subroutine RIGHT
userdiff.c +1 -1
index 707d82435a..fde02f225b 100644
@@ -53,7 +53,7 @@ IPATTERN("fortran",
 	 /* Program, module, block data */
 	 "^[ \t]*((END[ \t]+)?(PROGRAM|MODULE|BLOCK[ \t]+DATA"
 		/* Subroutines and functions */
-		"|([^'\" \t]+[ \t]+)*(SUBROUTINE|FUNCTION))[ \t]+[A-Z].*)$",
+		"|([^!'\" \t]+[ \t]+)*(SUBROUTINE|FUNCTION))[ \t]+[A-Z].*)$",
 	 /* -- */
 	 "[a-zA-Z][a-zA-Z0-9_]*"
 	 "|\\.([Ee][Qq]|[Nn][Ee]|[Gg][TtEe]|[Ll][TtEe]|[Tt][Rr][Uu][Ee]|[Ff][Aa][Ll][Ss][Ee]|[Aa][Nn][Dd]|[Oo][Rr]|[Nn]?[Ee][Qq][Vv]|[Nn][Oo][Tt])\\."
Signature · 1 line
gitgitgadget
#3 lore raw

Re: [PATCH 2/2] userdiff: improve Fortran xfuncname regex

Quoted 27 lines · Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>

The third part of the Fortran xfuncname regex wants to match the beginning of a subroutine or function, so it allows for all characters except ', " or whitespace before the keyword 'function' or 'subroutine'. This is meant to match the 'recursive', 'elemental' or 'pure' keywords, as well as function return types, and to prevent matches inside strings.

However, the negated set does not contain the ! comment character, so a line with an end-of-line comment containing the keyword 'function' or 'subroutine' followed by another word is mistakenly chosen as a hunk header.

Improve the regex by adding ! to the negated set.

Signed-off-by Philippe Blain <[email protected]>
---
 t/t4018/fortran-comment-keyword | 1 -
 userdiff.c                      | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
t/t4018/fortran-comment-keyword +0 -0
index c5dbdb4c61..e9206a5379 100644

t/t4018/fortran-comment-keyword line 13

Quoted 7 lines
t/t4018/fortran-comment-keyword +0 -1
@@ -8,7 +8,6 @@
       real funcB  ! grid function b
 
       real ChangeMe
-      integer broken
 
       end subroutine RIGHT

This change seems orthogonal to the explanation in the commit message. What is its purpose, and does it belong in this commit or a different one?

Quoted 15 lines
userdiff.c +1 -2
index 707d82435a..fde02f225b 100644
@@ -53,7 +53,7 @@ IPATTERN("fortran",
         /* Program, module, block data */
         "^[ \t]*((END[ \t]+)?(PROGRAM|MODULE|BLOCK[ \t]+DATA"
                /* Subroutines and functions */
-               "|([^'\" \t]+[ \t]+)*(SUBROUTINE|FUNCTION))[ \t]+[A-Z].*)$",
+               "|([^!'\" \t]+[ \t]+)*(SUBROUTINE|FUNCTION))[ \t]+[A-Z].*)$",
         /* -- */
         "[a-zA-Z][a-zA-Z0-9_]*"
         "|\\.([Ee][Qq]|[Nn][Ee]|[Gg][TtEe]|[Ll][TtEe]|[Tt][Rr][Uu][Ee]|[Ff][Aa][Ll][Ss][Ee]|[Aa][Nn][Dd]|[Oo][Rr]|[Nn]?[Ee][Qq][Vv]|[Nn][Oo][Tt])\\."
--

gitgitgadget

#4 lore raw

Re: [PATCH 2/2] userdiff: improve Fortran xfuncname regex

Hi Elijah,

Quoted 36 lines · Elijah Newren
Quoted 32 lines · Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>

The third part of the Fortran xfuncname regex wants to match the beginning of a subroutine or function, so it allows for all characters except ', " or whitespace before the keyword 'function' or 'subroutine'. This is meant to match the 'recursive', 'elemental' or 'pure' keywords, as well as function return types, and to prevent matches inside strings.

However, the negated set does not contain the ! comment character, so a line with an end-of-line comment containing the keyword 'function' or 'subroutine' followed by another word is mistakenly chosen as a hunk header.

Improve the regex by adding ! to the negated set.

Signed-off-by Philippe Blain <[email protected]>

---
t/t4018/fortran-comment-keyword | 1 -
userdiff.c | 2 +-

2 files changed, 1 insertion(+), 2 deletions(-)
t/t4018/fortran-comment-keyword +0 -1
index c5dbdb4c61..e9206a5379 100644
@@ -8,7 +8,6 @@
      real funcB  ! grid function b
 
      real ChangeMe
-      integer broken
Quoted 6 lines

t/t4018/fortran-comment-keyword line 13

Quoted 2 lines
t/t4018/fortran-comment-keyword +0 -0
      end subroutine RIGHT

This change seems orthogonal to the explanation in the commit message. What is its purpose, and does it belong in this commit or a different one?

If you take a look at t/t4018/README, the way to mark t4018 tests as "known failures" is to insert "broken" somewhere in the file. Since I'm fixing the regex in this commit to be able to cope with the situation in t/t4018/fortran-comment-keyword, I'm unmarking this test as broken.

Cheers,

Philippe.

#5 lore raw

Re: [PATCH 2/2] userdiff: improve Fortran xfuncname regex

Hi Philippe,

Quoted 46 lines · Philippe Blain

Hi Elijah,

Quoted 36 lines · Elijah Newren
Quoted 32 lines · Philippe Blain via GitGitGadget

From: Philippe Blain <[email protected]>

The third part of the Fortran xfuncname regex wants to match the beginning of a subroutine or function, so it allows for all characters except ', " or whitespace before the keyword 'function' or 'subroutine'. This is meant to match the 'recursive', 'elemental' or 'pure' keywords, as well as function return types, and to prevent matches inside strings.

However, the negated set does not contain the ! comment character, so a line with an end-of-line comment containing the keyword 'function' or 'subroutine' followed by another word is mistakenly chosen as a hunk header.

Improve the regex by adding ! to the negated set.

Signed-off-by Philippe Blain <[email protected]>

---
t/t4018/fortran-comment-keyword | 1 -
userdiff.c | 2 +-

2 files changed, 1 insertion(+), 2 deletions(-)
t/t4018/fortran-comment-keyword +0 -1
index c5dbdb4c61..e9206a5379 100644
@@ -8,7 +8,6 @@
      real funcB  ! grid function b
 
      real ChangeMe
-      integer broken
Quoted 6 lines

t/t4018/fortran-comment-keyword line 13

Quoted 2 lines
t/t4018/fortran-comment-keyword +0 -0
      end subroutine RIGHT

This change seems orthogonal to the explanation in the commit message. What is its purpose, and does it belong in this commit or a different one?

Quoted 3 lines

If you take a look at t/t4018/README, the way to mark t4018 tests as "known failures" is to insert "broken" somewhere in the file. Since I'm fixing the regex in this commit to be able to cope with the situation in t/t4018/fortran-comment-keyword, I'm unmarking this test as broken.

Ah, gotcha. I guess that's what I get for trying to review a random patch outside my area of expertise. :-) Thanks for explaining how this works to me.

Elijah

#6 lore raw
In reply to Philippe Blain via GitGitGadget
Quoted 2 lines · Philippe Blain via GitGitGadget

This series adds tests for the Fortran xfuncname regex and fixes a shortcoming of the regex.

Will queue. Thanks.

Quoted 33 lines

Philippe Blain (2):
userdiff: add tests for Fortran xfuncname regex
userdiff: improve Fortran xfuncname regex

 t/t4018/fortran-block-data          |  5 +++++
 t/t4018/fortran-comment             | 13 +++++++++++++
 t/t4018/fortran-comment-keyword     | 14 ++++++++++++++
 t/t4018/fortran-comment-legacy      | 13 +++++++++++++
 t/t4018/fortran-comment-legacy-star | 13 +++++++++++++
 t/t4018/fortran-external-function   |  9 +++++++++
 t/t4018/fortran-external-subroutine |  5 +++++
 t/t4018/fortran-module              |  5 +++++
 t/t4018/fortran-module-procedure    | 13 +++++++++++++
 t/t4018/fortran-program             |  5 +++++
 userdiff.c                          |  6 +++++-
 11 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 t/t4018/fortran-block-data
 create mode 100644 t/t4018/fortran-comment
 create mode 100644 t/t4018/fortran-comment-keyword
 create mode 100644 t/t4018/fortran-comment-legacy
 create mode 100644 t/t4018/fortran-comment-legacy-star
 create mode 100644 t/t4018/fortran-external-function
 create mode 100644 t/t4018/fortran-external-subroutine
 create mode 100644 t/t4018/fortran-module
 create mode 100644 t/t4018/fortran-module-procedure
 create mode 100644 t/t4018/fortran-program

base-commit: 4f0a8be78499454eac3985b6e7e144b8376ab0a5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-700%2Fphil-blain%2Ffortran-better-userdiff-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-700/phil-blain/fortran-better-userdiff-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/700