Re: [lmi] [lmi-commits] master 98a3720 2/6: Explicitly qualify std::size_t

[Vadim Zeitlin]

On Wed, 17 Feb 2021 12:59:13 +0000 Greg Chicares <gchicares@sbcglobal.net> 
wrote:

GC> On 2/17/21 11:11 AM, Vadim Zeitlin wrote:
[...]
GC> > Should we
GC> > (meaning I) add a coding rules check for this, i.e. verify that size_t is
GC> > always preceded by "std::" in all C++ files?
GC> > 
GC> >  Please let me know if you think this would be useful
GC> 
GC> No, I don't think so. Reasons, in order of increasing importance:
GC> 
GC> - It causes no immediate palpable harm, though of course a different
GC> compiler, or a new version of the same compiler, may reject it.

 Yes, there is no real harm, it's "just" about consistency. But aren't
many (most?) of the existing rules similar from this point of view?

GC> - False positives occur with almost any automated coding rule.

 This is true, but maybe I should have mentioned that I've actually tested
that

        $ git grep '[^:s]size_t.' *.?pp

finds no matches right now, so it looks like a check for this one would be
pretty simple. This wouldn't detect (even not commented out) occurrences of
size_t at the end of lines, but I don't think there is a real risk of this
happening in practice.

 And, FWIW, I'm going to add the above check to my own pre-commit hook
anyhow if we don't do it centrally.

GC> - Isn't it virtually always better to use 'int' instead of 'size_t'
GC> these days? If so, then it's virtually always a mistake to write
GC> 'size_t' at all. We've mostly replaced it with 'int' already, so
GC> the remaining occurrences are a code smell. For example, this
GC> declaration looks like a mistake:
GC> 
GC> // ABI:
GC> extern "C" char* __cxa_demangle
GC>     (char const * mangled_name  // mangled name, NUL-terminated
GC>     ,char       * output_buffer // just use 0
GC>     ,std::size_t* length        // just use 0
GC>     ,int        * status        // zero --> success
GC>     );
GC> 
GC> If we need a prototype for that, we should include some header.
GC> But AFAICS gcc doesn't require a prototype for __cxa_demangle
GC> because it's built in. I think I wrote that only as documentation,
GC> so it should be in a comment.

 Err, gcc definitely requires __cxa_demangle to have this prototype and on
the platforms where size_t and int have different size (such as Win64), it
would be a fatal mistake to use int here. Similarly, we still need to use
size_t when overriding base class virtual functions using it.

 So even though I agree with the beginning of this bullet point and would
use int rather than size_t in any new code, I don't think we can get rid of
it entirely in the observable future.

 Regards,
VZ

pgpnCqfU18tuT.pgp
Description: PGP signature