Is System.Reflection.TypeAttributes a sick enum type with the FlagsAttribute?

Question

The enum type System.Reflection.TypeAttributes appears rather pathological. It carries the [Flags] attribute and has no less than four synonyms for the constant zero. From Visual-Studio-generated "metadata":

#region Assembly mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089
// C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.2\mscorlib.dll
#endregion

using System.Runtime.InteropServices;

namespace System.Reflection
{
  //
  // Summary:
  //     Specifies type attributes.
  [ComVisible(true)]
  [Flags]
  public enum TypeAttributes
  {
    //
    // Summary:
    //     Specifies that the class is not public.
    NotPublic = 0,
    //
    // Summary:
    //     Specifies that class fields are automatically laid out by the common language
    //     runtime.
    AutoLayout = 0,
    //
    // Summary:
    //     Specifies that the type is a class.
    Class = 0,
    //
    // Summary:
    //     LPTSTR is interpreted as ANSI.
    AnsiClass = 0,

    // (followed by non-zero members...)

Why would anyone use four names for zero in an enum type which carries the FlagsAttribute? It seems really crazy.

Look at the consequences:

var x = default(System.Reflection.TypeAttributes);     // 0
var sx = x.ToString();                                 // "NotPublic"
var y = (System.Reflection.TypeAttributes)(1 << 20);   // 1048576
var sy = y.ToString();                                 // "AutoLayout, AnsiClass, Class, BeforeFieldInit"

Here the string representation of x, the zero value of the type, becomes "NotPublic". While the string representation of the non-zero y becomes "AutoLayout, AnsiClass, Class, BeforeFieldInit". Regarding y, note that it has only a single bit set (1<<20), and the name BeforeFieldInit alone accounts for that bit exactly. All the other three names, AutoLayout, AnsiClass, Class,, contribute with zero to the value.

What is going on?

Why this design?

These names come from the CLI spec, Ecma 335. They made the enum definition as close as possible to the spec, nothing very sick about that. Assembly metadata is excessively compact first, bit-packing as many options in a field as they could fit. Having a bit not set now matters as well, they never tried to make it friendly. It only ever matters to very low-level assembly dumping code, not the kind of code that blindly depends on ToString(). — Hans Passant, Oct 07 '16 at 15:11

score 3 · Accepted Answer · answered Oct 07 '16 at 14:44

3

The ToString() representation is largely irrelevant

This pattern is pretty common when some of the options are non-binary; for example, there are 3 possible options. In that scenario you might designate 2 bits to carry those 3 options (leaving 4 unused), and the "default" option would be (logically) 00. This means that yes, there will be multiple synonyms for 0.

Note: this might also happen in purely binary options, if the enum author wants to make it more explicit - because the caller doesn't need to know which are "on" and which are "off".

Basically, don't worry about ToString()

answered Oct 07 '16 at 14:44

Marc Gravell

1,026,079
266
2,566
2,900

What does `AutoLayout` mean, just to pick an example? If I test `typeof(TimeSpan).Attributes.HasFlag(TypeAttributes.AutoLayout)`, the result is true. So `TimeSpan` has layout "auto", right? Also `typeof(TimeSpan).Attributes.HasFlag(TypeAttributes.SequentialLayout)` is true as well, so `TimeSpan` has layout sequential as well, right? – Jeppe Stig Nielsen Oct 07 '16 at 14:49
@JeppeStigNielsen (note: deleted previous) I think the problem there is simply `ToString()`. Again, this is a triplet - auto, explicit or sequential. `ToString()` gets it wrong, as does the naive test. The *correct* test here is `val & 24` - is that 0 (`AutoLayout`), 8 (`SequentialLayout`) or 16 (`ExplicitLayout`)? `HasFlag` is only designed for binary flags, which this one isn't. IMO this is a "95%" thing. The API you are using is designed for the 95% care which isn't this. (actually, there's also `LayoutMask` = 24, which is a special case of explicit, so: kinda 3.5 options expressed) – Marc Gravell Oct 07 '16 at 14:59
How will I know that "auto" is the magical case of those three? They could have chosen explicit=0, sequential=8, auto=16 just as well? In that case, I would not be able test for "explicit" directly (because the value is zero). They do have a name `LayoutMask` for the sum `8 | 16`. – Jeppe Stig Nielsen Oct 07 '16 at 15:03
@JeppeStigNielsen hah, yeah, it looks like I misunderstood the `LayoutMask` one and that is designed purely for the exact test you're asking about. As for how to know that it isn't a binary flag-set; I guess docs, but not very clear in this case. As it happens I wrote one of these violations yesterday - I hope my comments are better than MSDN's ! https://github.com/mgravell/Channels.Http2/blob/master/src/Channels.Http2/Hpack.cs (see: `public enum HeaderOptions`). – Marc Gravell Oct 07 '16 at 15:07
To me, the only sensible thing would be to use one bit for each of the three options, as in e.g. `AutoLayout=4, SequentialLayout = 8, ExplicitLayout = 16,`. You could still have names for the combinations ("masks") `4 | 8` etc. etc. if you wanted. If I read your answer above I get the impression that in your opinion there is nothing wrong with the design here? Do you really think that? – Jeppe Stig Nielsen Oct 07 '16 at 15:11
Yes, I agree with the enum and disagree with your assertion. Because they **aren't binary flags**. Sequential | Explicit is not legal, for example. There is also a practical consideration of space, expecially when the options in a particular segment are large - say, 7 options. That can be expressed in 3 bits, but you would have it take 7. That type of expansion could get problematic for busy enums. – Marc Gravell Oct 07 '16 at 15:17
@JeppeStigNielsen revised example - I think it is better than it was before this discussion: https://github.com/mgravell/Channels.Http2/blob/master/src/Channels.Http2/HeaderOptions.cs – Marc Gravell Oct 07 '16 at 15:19
OK, I am beginning to see what the design is. For example they use the three least significant bits as a "word" to specify the accessibility (2³ possibilities), and so the fact that `Public | NestedFamily == NestedAssembly` should not be "used". Those three bits should be seen as a whole. And similarly for other positions. – Jeppe Stig Nielsen Oct 07 '16 at 15:26
@JeppeStigNielsen exactly; they are packing multiple non-binary options into a single `[Flags]` enum for efficiency reasons. The alternative would be to have multiple non-`[Flags]` enums, one for each family - so a `CustomFormat` option, a `Layout` option, a `ClassSemantics` option, a `StringFormat` option and a `Visibility` option. But what they have works fine too, IMO. – Marc Gravell Oct 07 '16 at 15:30

Is System.Reflection.TypeAttributes a sick enum type with the FlagsAttribute?

1 Answers1