Re: [lmi] PATCH: miscellaneous improvements to test_coding_rules

[Vadim Zeitlin]

On Mon, 28 Jun 2021 22:43:58 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 2021-06-17 21:06, Vadim Zeitlin wrote:
GC> > On Wed, 16 Jun 2021 00:25:36 +0000 Greg Chicares 
<gchicares@sbcglobal.net> wrote:
GC> [...]
GC> > GC> Okay, I'm ready for that whenever you like.
GC> 
GC> [...modulo twelve days...]

 Thanks for taking time to look at this!

GC> I have some "BTW" comments about two commits, identified here by
GC> their savannah SHA1s:
GC> 
GC> 0f1bc0773 Split the check for canonical header guards in two parts
GC> 
GC> The old and new revisions behave identically AFAICS. Both test thus:
GC> 
GC>   // any lines at all
GC>   opening include guard
GC>   // any lines at all; next line must be the last:
GC>   closing include guard
GC> 
GC> An enhancement would be to forbid anything interesting before the
GC> opening include guard:
GC> 
GC>   // boilerplate C++ comments, blank lines: okay
GC>   int xyzzy = 42; // NOT OKAY
GC>   opening include guard [etc.]
GC> 
GC> in case you feel inspired to implement that (me, I'd rather return
GC> to working on inverse quadratic interpolation).

 I could do this, but I really need to finish my PCRE-related changes first
and unfortunately I haven't done it at all yet, so I'd rather concentrate
on this for now.

GC> 30ba88acc Remove check for Latin-9 from coding rules test
GC> 
GC> I can now write
GC>   6ϵ|ζ|
GC> in comments, which is nicer than
GC>   6.0 * std::numeric_limits<double>::epsilon() * std::fabs(zeta)
GC> so this is a welcome change. I did worry about no longer filtering
GC> out bizarre invisible characters that may occur in corporate
GC> documents, but 'vim' makes them visible, e.g. as follows:
GC>   Auf<200c>lage // zero-width non-joiner
GC>   Lorem​<200b>Ipsum // zero-width space
GC>   '<180e>᠎' // Mongolian vowel separator
GC> so I guess there's nothing to worry about after all.

 We could write a specific test checking for the unwanted characters. PCRE
provides syntax to test for Unicode properties, so we could just search for
the characters in the "Other, Format" (Cf) category to which all of the
above ones belong. And I think this one could actually be quite useful,
because even if such characters are visible in Vim, they can still result
in a waste of time if they get into the source code as gcc doesn't like
them at all (as you can imagine, I'm speaking from personal experience
here).

GC> >, and while I do have some doubts about 0790ce22d (Move
GC> > statistics display from test_coding_rules to makefile, 2021-06-02), I've
GC> > already expressed them in the commit message itself.
GC> 
GC> I dropped that commit, because it changed the behavior:
GC> 
GC> original:
GC>       692 source files
GC>    197000 source lines
GC>       276 marked defects
GC> 
GC> with that commit:
GC>         1 source files
GC>   6684988 lines
GC>      1076 defects

 Oops, sorry, I don't know how I didn't notice this. I'll return to this
slightly later, but I'd just like to confirm that at least you agree with
moving the statistics display to the makefile in principle. And, of course,
if you have a better idea for avoiding duplicating the check for
"significant" files between test_coding_rules.cpp and the makefile, I'd be
glad to hear it.

 Thanks again for merging this!
VZ

pgpwWdAx3KIAY.pgp
Description: PGP signature