Re: [PATCH #2/4] c++: mark short-enums as packed

[Alexandre Oliva <oliva@adacore.com>]

On Nov 20, 2023, Jason Merrill <jason@redhat.com> wrote:

> I think the warning is wrong here.

Interesting...  Yeah, your analysis makes perfect sense.

Still, we're left with a divergence WRT the TYPE_PACKED status of enum
types between C and C++.

It sort of kind of makes sense to mark short enums as packed, because,
well, they are.

Even enum types with explicit attribute packed, that IIUC uses the same
underlying type selection as -fshort-enums, IIRC are not be marked with
TYPE_PACKED in C++, at least not at the place where I proposed to set
it.  Do you consider that behavior correct?

Even if the warning happens to be buggy in this regard, it is at best
(or worst) accessory to this patch, in that it makes that difference
between languages apparent, and I worry that there might be other middle
end tests involving TYPE_PACKED that would get things different in C vs
C++.  (admittedly, I haven't searched for occurrences of TYPE_PACKED in
the tree, but I could, to alleviate my concerns, in case there's a
decision to keep them different)

> In the analyzer testcase, we have a cast from an
> enum pointer that we don't know what it points to, and even if it did 
> point to the obj_type member of struct connection, that wouldn't be a
> problem because it's at offset 0.

Maybe I misunderstand the point of the warning, but ISTM that the
circumstance it's warning about is real: the member is not as aligned as
the enclosing struct, so the cast is risky.  Now, I suppose the idiom of
finding the enclosing struct given a member is common enough that we
don't want to warn about it in general.  I'm not sure what makes packed
structs special in this regard, though.  I don't really see much
difference, more laxly-aligned fields seem equally warn-worthy, whether
the enclosing struct is packed or not, but what do I know?

> Also, -fshort-enums has nothing to do with structure packing

*nod*, it's about packing of the enum type itself.  It is some sort of a
degenerated aggregate type ;-) But yeah, I guess it doesn't fit the
circumstance the warning was meant to catch, and the fact that in C is
does is a consequence of marking C short enums as TYPE_PACKED.

Which might be a bug in C.

But wouldn't it be a bug in C++ if an enum with attribute packed weren't
markd as TYPE_PACKED?  Or is TYPE_PACKED really meant to say something
about the enclosing struct rather than about the enclosed type itself?
(am I getting too philosophical here? :-)


Thanks,

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive