[clang-tidy] Fix `altera-struct-pack-align` check for empty structs

[fwolff]

Summary

Fixes PR#51620.

[whisperity]

I believe this is worth a note in the ReleaseNotes.rst file. People who might have disabled the check specifically for the reason outlined in the PR would get to know it's safe to reenable it.

clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp

81

If we are changing this, can we make this more C++-y?

[aaron.ballman]

+1 to the request for a release note, but otherwise this LGTM (with or without the static_cast changes) in general. Should we also do something special for [[no_unique_address]] (if we should, I'm fine doing that in a follow-up)?

[fwolff]

I've added the static_cast and an entry in the release notes. I'm not sure how to handle [[no_unique_address]], so I'd rather leave this to future work.

[whisperity]

@aaron.ballman Alright, I think this can go. The ReleaseNotes.rst needs a rebase anyway.

[aaron.ballman]

Oh, yeah, this fell off my radar. Agreed, LGTM!

clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp

for (const FieldDecl *StructField : Struct->fields()) {
     // FIXME: Recommend a reorganization of the struct (sort by StructField
     // size, largest to smallest).
     TotalBitSize += StructFieldWidth;
   }
 
   uint64_t CharSize = Result.Context->getCharWidth();
   CharUnits CurrSize = Result.Context->getASTRecordLayout(Struct).getSize();
   CharUnits MinByteSize =
-      CharUnits::fromQuantity(ceil((float)TotalBitSize / CharSize));
+      CharUnits::fromQuantity(std::max<clang::CharUnits::QuantityType>(
+          ceil(static_cast<float>(TotalBitSize) / CharSize), 1));
   CharUnits MaxAlign = CharUnits::fromQuantity(
       ceil((float)Struct->getMaxAlignment() / CharSize));
   CharUnits CurrAlign =
       Result.Context->getASTRecordLayout(Struct).getAlignment();
   CharUnits NewAlign = computeRecommendedAlignment(MinByteSize);
 
   bool IsPacked = Struct->hasAttr<PackedAttr>();
   bool NeedsPacking = (MinByteSize < CurrSize) && (MaxAlign != NewAlign) &&

clang-tools-extra/docs/ReleaseNotes.rst

 - Fixed a false positive in :doc:`readability-non-const-parameter
   <clang-tidy/checks/readability-non-const-parameter>` when the parameter is
   referenced by an lvalue.
 
 - Improved :doc:`performance-inefficient-vector-operation
   <clang-tidy/checks/performance-inefficient-vector-operation>` to work when
   the vector is a member of a structure.
 
+- Fixed nonsensical suggestion of :doc:`altera-struct-pack-align
+  <clang-tidy/checks/altera-struct-pack-align>` check for empty structs.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/altera-struct-pack-align.cpp

struct success4 {
   int c;
 };
 
 // Warnings should not trigger on struct instantiations
 void no_trigger_on_instantiation() {
   struct bad_align3 instantiated { 'a', 0.001, 'b' };
 }
 
+// Make sure that we don't recommend aligning an empty struct to zero bytes (PR#51620)
+struct StructWithNoFields {};
+
+struct ContainsStructWithNoFields {
+  StructWithNoFields s;
+};
+
+// Make sure that an empty struct is treated like "char" for padding and alignment purposes
+struct ContainsStructWithNoFields2 {
+  StructWithNoFields s;
+  double d;
+  StructWithNoFields t;
+};
+// CHECK-MESSAGES: :[[@LINE-5]]:8: warning: accessing fields in struct 'ContainsStructWithNoFields2' is inefficient due to padding; only needs 10 bytes but is using 24 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-6]]:8: note: use "__attribute__((packed))" to reduce the amount of padding applied to struct 'ContainsStructWithNoFields2'
+// CHECK-MESSAGES: :[[@LINE-7]]:8: warning: accessing fields in struct 'ContainsStructWithNoFields2' is inefficient due to poor alignment; currently aligned to 8 bytes, but recommended alignment is 16 bytes [altera-struct-pack-align]
+// CHECK-MESSAGES: :[[@LINE-8]]:8: note: use "__attribute__((aligned(16)))" to align struct 'ContainsStructWithNoFields2' to 16 bytes
+// CHECK-FIXES: __attribute__((packed))
+// CHECK-FIXES: __attribute__((aligned(16)));