Skip to content

Conversation

@DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented Sep 21, 2021

Unmatchable caret in regular expression
This regular expression includes an unmatchable caret at offset 18.

https://lgtm.com/projects/g/numpy/numpy/snapshot/8b1ae13424d3f203fb21b3063a03b80358fadfcf/files/numpy/linalg/lapack_lite/clapack_scrub.py#x56fba2496b1bf29c:1

Indeed this regex couldn't possibly match anything.

Note that this regex seems obsolete, it attempts to match "inline" or "static" specifiers, which I cannot find in the C files generated by f2c from LAPACK. Typical declarations look like this:

    extern /* Subroutine */ int clahqr_(logical *, logical *, integer *,
            integer *, integer *, complex *, integer *, complex *, integer *,
            integer *, complex *, integer *, integer *), clacpy_(char *,
            integer *, integer *, complex *, integer *, complex *, integer *);

EDIT: Removed any reference to CLAPACK, it was confusing.

@eric-wieser
Copy link
Member

eric-wieser commented Sep 21, 2021

which I cannot find in the sources of CLAPACK 3.2.1

What are you referring to by CLAPACK? An upstream set of lapack bindings, or the one in question here? Numpy's lapack_lite uses a scripted translation of the original fortran code via f2c and some manual patching, which as far as I'm aware is where this script comes in.

@DimitriPapadopoulos
Copy link
Contributor Author

From README.rst:

You'll need plex 2.0.0dev, available from PyPI, installed to do the appropriate scrubbing. As of writing, this is only available for python 2.7, and is unlikely to ever be ported to python 3.

I'm puzzled, since make_lite.py is a Python 3 script, how is this supposed to work?

@eric-wieser
Copy link
Member

I'm puzzled, since make_lite.py is a Python 3 script,

My guess was that it was blindly converted to Python3 without thought, and has not been run for a long time.

@DimitriPapadopoulos
Copy link
Contributor Author

Should make_lite.py be reverted to Python 2.7?

@eric-wieser
Copy link
Member

Anything that puts make_lite.py back in a state that actually runs (and outputs the same files that are currently checked in) would be appreciated!

@DimitriPapadopoulos
Copy link
Contributor Author

Oh, the files themselves had been regenerated back in 2020 (#15224). I'll try to revert to that version instead.

@eric-wieser
Copy link
Member

I think any changes which added Python 3 support are fine (in case anyone ever decides to port plex in future), it's the ones that removed Python 2 support which were in error. Certainly we need a docstring indicating that these are python2 scripts!

@DimitriPapadopoulos
Copy link
Contributor Author

Fine. Also, I suggest changing the shebang from #!/usr/bin/env python3 to #!/usr/bin/env python2.7 to make it crystal clear this is a Python 2 script.

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Sep 21, 2021

Removing the caret from the regex does generate some matches. Here are the diffs:

diff --git a/numpy/linalg/lapack_lite/f2c_blas.c b/numpy/linalg/lapack_lite/f2c_blas.c
index 65286892f..ceb9ba0b4 100644
--- a/numpy/linalg/lapack_lite/f2c_blas.c
+++ b/numpy/linalg/lapack_lite/f2c_blas.c
@@ -4008,7 +4008,6 @@ L20:
     return 0;
 } /* csrot_ */
 
-/* Subroutine */ int csscal_(integer *n, real *sa, complex *cx, integer *incx)
 {
     /* System generated locals */
     integer i__1, i__2, i__3, i__4;
@@ -12424,7 +12423,6 @@ L20:
     return 0;
 } /* srot_ */
 
-/* Subroutine */ int sscal_(integer *n, real *sa, real *sx, integer *incx)
 {
     /* System generated locals */
     integer i__1, i__2;
diff --git a/numpy/linalg/lapack_lite/f2c_c_lapack.c b/numpy/linalg/lapack_lite/f2c_c_lapack.c
index c36c0e368..37cc19630 100644
--- a/numpy/linalg/lapack_lite/f2c_c_lapack.c
+++ b/numpy/linalg/lapack_lite/f2c_c_lapack.c
@@ -8875,7 +8875,6 @@ L10:
 
 } /* clabrd_ */
 
-/* Subroutine */ int clacgv_(integer *n, complex *x, integer *incx)
 {
     /* System generated locals */
     integer i__1, i__2;
diff --git a/numpy/linalg/lapack_lite/f2c_config.c b/numpy/linalg/lapack_lite/f2c_config.c
index 3f59e0263..d40c1d63f 100644
--- a/numpy/linalg/lapack_lite/f2c_config.c
+++ b/numpy/linalg/lapack_lite/f2c_config.c
@@ -727,7 +727,6 @@ doublereal dlamc3_(doublereal *a, doublereal *b)
 
 /* *********************************************************************** */
 
-/* Subroutine */ int dlamc4_(integer *emin, doublereal *start, integer *base)
 {
     /* System generated locals */
     integer i__1;
@@ -1797,7 +1796,6 @@ doublereal slamc3_(real *a, real *b)
 
 /* *********************************************************************** */
 
-/* Subroutine */ int slamc4_(integer *emin, real *start, integer *base)
 {
     /* System generated locals */
     integer i__1;
diff --git a/numpy/linalg/lapack_lite/f2c_d_lapack.c b/numpy/linalg/lapack_lite/f2c_d_lapack.c
index 233db74b9..bb2412b3c 100644
--- a/numpy/linalg/lapack_lite/f2c_d_lapack.c
+++ b/numpy/linalg/lapack_lite/f2c_d_lapack.c
@@ -7965,7 +7965,6 @@ logical disnan_(doublereal *din)
     return ret_val;
 } /* disnan_ */
 
-/* Subroutine */ int dlabad_(doublereal *small, doublereal *large)
 {
 
 /*
@@ -29214,7 +29213,6 @@ L100:
 
 } /* dlasq1_ */
 
-/* Subroutine */ int dlasq2_(integer *n, doublereal *z__, integer *info)
 {
     /* System generated locals */
     integer i__1, i__2, i__3;
diff --git a/numpy/linalg/lapack_lite/f2c_s_lapack.c b/numpy/linalg/lapack_lite/f2c_s_lapack.c
index 2a32315c7..8333e6c04 100644
--- a/numpy/linalg/lapack_lite/f2c_s_lapack.c
+++ b/numpy/linalg/lapack_lite/f2c_s_lapack.c
@@ -7945,7 +7945,6 @@ logical sisnan_(real *sin__)
     return ret_val;
 } /* sisnan_ */
 
-/* Subroutine */ int slabad_(real *small, real *large)
 {
 
 /*
@@ -8562,7 +8561,6 @@ logical sisnan_(real *sin__)
 
 } /* sladiv_ */
 
-/* Subroutine */ int slae2_(real *a, real *b, real *c__, real *rt1, real *rt2)
 {
     /* System generated locals */
     real r__1;
@@ -23399,7 +23397,6 @@ L410:
 
 } /* slarfx_ */
 
-/* Subroutine */ int slartg_(real *f, real *g, real *cs, real *sn, real *r__)
 {
     /* System generated locals */
     integer i__1;
@@ -29103,7 +29100,6 @@ L100:
 
 } /* slasq1_ */
 
-/* Subroutine */ int slasq2_(integer *n, real *z__, integer *info)
 {
     /* System generated locals */
     integer i__1, i__2, i__3;
@@ -31213,7 +31209,6 @@ L80:
 
 } /* slasr_ */
 
-/* Subroutine */ int slasrt_(char *id, integer *n, real *d__, integer *info)
 {
     /* System generated locals */
     integer i__1, i__2;
@@ -38357,7 +38352,6 @@ L190:
 
 } /* ssteqr_ */
 
-/* Subroutine */ int ssterf_(integer *n, real *d__, real *e, integer *info)
 {
     /* System generated locals */
     integer i__1;
diff --git a/numpy/linalg/lapack_lite/f2c_z_lapack.c b/numpy/linalg/lapack_lite/f2c_z_lapack.c
index 8234eca41..e5a41e64c 100644
--- a/numpy/linalg/lapack_lite/f2c_z_lapack.c
+++ b/numpy/linalg/lapack_lite/f2c_z_lapack.c
@@ -8930,7 +8930,6 @@ L10:
 
 } /* zlabrd_ */
 
-/* Subroutine */ int zlacgv_(integer *n, doublecomplex *x, integer *incx)
 {
     /* System generated locals */
     integer i__1, i__2;

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Sep 21, 2021

This regex was introduced in #10477, with the caret.

It was supposed to "align type definitions", not sure what that means.

Any clue whether the declarations of csscal_(), sscal_(), clacgv_(), dlamc4_() and the rest should indeed be removed?

Unmatchable caret in regular expression
This regular expression includes an unmatchable caret at offset 18.

Indeed this regex couldn't possibly match anything. Not only does the
regex look obsolete, attempting to match "inline" or "static" specifiers
that I cannot find in the sources produced by 'f2c', but the very logic
of function removeSubroutinePrototypes() makes it impossible to handle
'/* Subroutine */' declarations that span multiple lines.

All in all, function removeSubroutinePrototypes() does nothing at all.
We choose not to change that, because it works and we have no real hint
what it should do and why. We just document that.
@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented Sep 21, 2021

I have sort of discarded removeSubroutinePrototypes(). This function was doing nothing at all, almost by design, despite its name. Keep it that way - but document the fact and remove the broken and useless code.

@DimitriPapadopoulos DimitriPapadopoulos changed the title MAINT: Fix LGTM.com error MAINT: Fix LGTM.com error: Unmatchable caret in regular expression Sep 24, 2021
@charris
Copy link
Member

charris commented Sep 27, 2021

@DimitriPapadopoulos Yes, removeSubroutinePrototypes() actually seems to do the wrong thing when "corrected".

@charris charris merged commit 00dce98 into numpy:main Sep 27, 2021
@charris
Copy link
Member

charris commented Sep 27, 2021

Thanks @DimitriPapadopoulos .

@DimitriPapadopoulos DimitriPapadopoulos deleted the lgtm_error branch September 27, 2021 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants