On Nov 20, 2023, Jason Merrill <ja...@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
