aaron.ballman added inline comments.
================
Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:66-68
+ // FIXME: Express this as CharUnit rather than a hardcoded 8-bits (Rshift3)i
+ // After computing the minimum size in bits, check for an existing alignment
+ // flag.
----------------
I'm not certain I understand this FIXME, is it out of date? You're getting the
width of a char and using that rather than hard-coding already.
================
Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:71
+ CharUnits MinByteSize =
+ // CharUnits::fromQuantity(ceil((float)TotalBitSize / CHAR_BIT));
+ CharUnits::fromQuantity(ceil((float)TotalBitSize / CharSize));
----------------
This should probably be removed.
================
Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:85
+ ((Struct->getMaxAlignment() / CharSize) != NewAlign.getQuantity()) &&
+ // ((Struct->getMaxAlignment() / CHAR_BIT) != NewAlign.getQuantity()) &&
+ (CurrSize != NewAlign) && !IsPacked) {
----------------
This should also probably be removed.
================
Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:93-94
+ diag(Struct->getLocation(),
+ "use \"__attribute__((packed))\" to reduce the amount of padding "
+ "applied to struct %0", DiagnosticIDs::Note)
+ << Struct;
----------------
Would it make sense to generate a fixit to add the attribute to the struct so
long as the struct is not in a system header?
================
Comment at: clang-tidy/altera/StructPackAlignCheck.cpp:107
+ diag(Struct->getLocation(),
+ "use \"__attribute__((aligned(%0)))\" to align struct %1 to %0 bytes",
+ DiagnosticIDs::Note)
----------------
Similar question here about the fixit.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66564/new/
https://reviews.llvm.org/D66564
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
