[RFC] The coding standard for "struct" should be relaxed or removed

The current guidelines [1] on the use of the struct keyword are too
restrictive and apparently ignored. They limit the use of struct to
PODs, citing broken compilers.

The guidelines are out-of-date and should be relaxed. Here’s why:

1. Our updated list of supported compilers should all deal correctly
    with non-POD structs.

2. A quick grep of the source suggests that no one paid attention
    anyway.

I’ve attached a patch that removes the guideline entirely (matching
the apparent current practice), but does anyone feel a new explicit
guideline is in order?

[1]: http://llvm.org/docs/CodingStandards.html#use-of-class-and-struct-keywords

remove-struct-guidelines.patch (1.19 KB)

We should at least remove it, the rule is nonsense as you point out.

I would be happy to have a soft rule along the lines of "use struct for
types which are intended to be aggregate types, or to behave like aggregate
types". IE, types which are suitable for use with list-initialization in
C++11.

I also don't feel terribly strongly about such things other than that I
would prefer *some* guidance to lessen the inconsistency.

This rule was specifically about brokenness with some version of MSVC. That version (I have no idea which, or if it still does that) mangled classes and structs differently. If this isn’t the case for the currently supported version of MSVC, we should definitely remove this guideline.

-Chris

MSVC still mangles the tag into the type, making the manglings for class
and struct differ. We even have code for this in MicrosoftMangle.cpp. The
consequence is that you have to use the right tag when you forward declare
something, which we can spot with -Wmismatched-tags. I don't think this is
a very big deal, and we can probably remove the rule.

According to <http://www.agner.org/optimize/calling_conventions.pdf&gt;
(see table 9), MSVC still mangles classes and structs differently.

Nevertheless, the rule is not being followed. E.g., df_ext_iterator,
APInt::ms, ilist_traits<SparseBitVectorElement<ElementSize>>, and
CaptureTracker are four quite different examples that run afoul (none
of these is POD). It’s easy to get this wrong; there are many more
examples.

I’m not even sure the rule is helpful. Even POD types can be
alternately declared “class” or “struct”, changing their mangling in
MSVC.

I’ve attached a patch that clarifies the mangling problem and
describes the actual practice in the codebase. How does this look?

clarify-struct-guidelines.patch (1.39 KB)

Works for me,

-Chris

Committed in r202728.