rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2029-2036
// The align if the field is not packed. This is to check if the attribute
// was unnecessary (-Wpacked).
CharUnits UnpackedFieldAlign =
!DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
CharUnits UnpackedFieldOffset = FieldOffset;
CharUnits OriginalFieldAlign = UnpackedFieldAlign;
----------------
It seems a little wasteful and error-prone that we're now computing the actual
alignment, the alignment if the field were not packed, and the alignment if the
field were packed. Is there any way we can reduce this down to computing just
the alignment if the field were packed plus the alignment if the field were not
packed, then picking one of those two as the actual field alignment? Or does
that end up being messier?
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2140-2141
+
+ if (Packed && !FieldPacked && PackedFieldAlign < FieldAlign)
+ Diag(D->getLocation(), diag::warn_unpacked_field) << D;
}
----------------
Hm. Doing this from here will mean we only warn if we actually compute the
layout of the class. But I suppose that's the same as what we do a few lines
above, and for other `-Wpacked` warnings, and it seems necessary if we want to
suppress the warning if packing wouldn't have made any difference anyway.
================
Comment at: clang/test/CodeGenCXX/warn-padded-packed.cpp:157
+ char c2;
+ S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is
non-POD}}
+} __attribute__((packed));
----------------
Maybe consider adding another test showing that we don't warn if packing would
have made no difference anyway (for example if the alignment of the POD struct
was already 1).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118511/new/
https://reviews.llvm.org/D118511
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
