A couple of GCC's Coding Conventions call to

  1) Use the struct keyword for plain old data (POD) types.
     https://www.gnu.org/software/gcc/codingrationale.html#struct

and

  2) Use the class keyword for non-POD types.
     https://www.gnu.org/software/gcc/codingconventions.html#Class_Use

The rationale for the conventions is to convey information about
some basic properties of a user-defined type such as whether it
is safely and efficiently copyable or assignable via memcpy or
whether it requires a call to a copy ctor/assignment operator
or destructor.

A survey of GCC code shows that the convention is only loosely
followed, making it less helpful than it would be otherwise.
In addition, inconsistencies between declarations and definitions
are wide-spread.  Relying on the convention can lead to using
a non-POD class that's declared using the struct class-key in
contexts that aren't prepared for such a type (although not
a direct result, pr90904, 90923, 90959 are examples of some of
the bugs that this might lead to).

The poor consistency with which the convention is adhered to makes
it easy for authors to miss that it exists.  This can then lead to
inefficiencies in code reviews both for reviewers and for authors.

In addition, Clang users who build GCC are bothered by instances
of Clang's -Wmismatched-tags warning (see pr61339 and the recent
patch: https://gcc.gnu.org/ml/gcc-patches/2019-05/msg01805.html).
Clang includes -Wmismatched-tags in -Wall even on Linux.  Although
-Wmismatched-tags is provided for compatibility with Visual C++
(and there to help avoid a potential ABI problem), the warning
points out the same lack of consistency in the GCC coding
convention: the type being sometimes declared as a struct and
other times as a class.

To make the code base consistent with the convention and enable
relying on the struct and class keywords having the intended
meaning, to reduce the time spent in code reviews on details
that an easily be caught earlier by out tools, and to avoid
the -Wmismatched-tags warnings when using Clang, the attached
patch kit does three things:

1) Implements three new warnings:
   -Wstruct-not-pod triggers for struct definitions that are not
   POD structs,
   -Wclass-is-pod triggers for class definitions that satisfy
   the requirements on POD structs,
   and -Wmismatched-tags that triggers for class and struct
   declarations with class-key that doesn't match either their
   definition or the first declaration (if no definition is
   provided).  All three warnings have to be explicitly enabled
   to have an effect.
2) Makes changes to definitions of GCC classes and structs to
   avoid instances of the three new warnings.
3) Changes declarations of GCC classes and structs to match
   the class-key used in their definition.

If/when the patch kit is approved I will enable the three
warnings for GCC builds.  I expect to take the same approach as
with -Wformat-diag: first prevent any remaining instances of
the warnings from triggering errors, clean up or suppress
the remaining instances, and finally enable them to cause errors
with -Werror.

Reply via email to