Re: [lmi] Branch lmi-so

[Vadim Zeitlin]

On Sun, 18 Oct 2020 14:55:58 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 2020-10-18 12:47, Greg Chicares wrote:
GC> > This branch generally moves "LMI_SO" to the left, e.g.:
GC> > 
GC> > -void LMI_SO authenticate_system();
GC> > +LMI_SO void authenticate_system();
GC> > 
GC> > but in 'basic_tables.cpp', "LMI_SO" is removed, e.g.:
GC> > 
GC> > -std::vector<double> LMI_SO irc_7702_q_builtin
GC> > +std::vector<double> irc_7702_q_builtin
GC> > 
GC> > Is that intentional, and if so, what's the reason?
GC> 
GC> Oh, wait--I think I see it now. "LMI_SO" is removed from
GC>  - definitions in '.cpp' files, where it already decorates the corresponding

 Yes, sorry, this was supposed to be mentioned in the cover letter for this
PR, which I've never written. But LMI_SO is indeed useless on the
definitions and is already not used in 90% of the cases, so the PR just
removes it from the few functions for which it is used, for whatever
reason.

GC>  - declarations in '.hpp' files.

 I don't think the PR removes LMI_SO from the declarations in the headers,
but perhaps I've missed something in my own review.

GC> This all looks perfect to me now. BTW, I checked it by running the
GC> following command, which moves "LMI_SO" leftward, reading from my
GC> convenient local mirror of origin/master and writing to a newly
GC> created throwaway directory:
GC> 
GC> /opt/lmi/free/src/lmi[0]$for z in `grep -l -P '(?<!class )LMI_SO' *.?pp`; 
do <$z >/opt/lmi/stash/so/$z sed -e'/\(class\|struct\|extern\) 
*LMI_SO/!s/\(^.*\)\(LMI_SO \)\(.*$\)/\2\1\3/'; done
GC> 
GC> Negative lookbehind isn't supported by 'sed', but a negated range
GC> accomplishes the same thing.

 Sorry, I was going to write a Perl oneliner doing this before submitting
the PR to you, but you've beaten me to it...

GC> Concerning the commit below, I'd just like to make an observation:
GC> apparently the "SO" decoration for forward declarations is...
GC>   #define LMI_SO_FWD_DECL        // GNU/Linux current gcc
GC>   #define LMI_SO_FWD_DECL        // MinGW-w64 since we began using it
GC>   #define LMI_SO_FWD_DECL LMI_SO // historical mingw.org gcc
GC> if I understand it now (after misunderstanding on the first few
GC> readings); therefore, the preceding documentation:
GC> 
GC> // [...] It is unknown
GC> // whether this difference represents deliberate evolution of gcc or
GC> // a MinGW-w64 regression, so both versions are preserved.
GC> 
GC> can now be revised (it really is evolution, and perhaps mingw.org
GC> was just lax in accepting the decoration); and in fact we could
GC> simplify it from this:
GC> 
GC> #   if defined __GNUC__
GC> #       if defined LMI_MINGW_W64
GC> #           define LMI_SO_FWD_DECL
GC> #       else  // !defined LMI_MINGW_W64
GC> #           define LMI_SO_FWD_DECL
GC> #       endif // !defined LMI_MINGW_W64
GC> #   else  // !defined __GNUC__
GC> 
GC> to something like this:
GC> 
GC> #   if defined __GNUC__
GC> #       define LMI_SO_FWD_DECL
GC> #   else  // !defined __GNUC__

 Yes. The question is whether we should remove LMI_SO_FWD_DECL completely
or still keep it. Personally I think we should remove it, but I've decided
not to do it for now to avoid having even more changes in this PR.

 Thanks again for looking at this!
VZ

pgpjMhlHV2VUy.pgp
Description: PGP signature