[PATCH] D14779: Adding checker to detect excess padding in records

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

With respect to the issues this checker found, I suggest submitting patches for the clang issues that can be fixed. Can the x-macro case be suppressed with the recommended suppression? I'd also submit a patch to gtest. Submitting patches with the fixes provides a good evaluation of new checkers:)


================
Comment at: lib/StaticAnalyzer/Checkers/Checkers.td:49
@@ -48,1 +48,3 @@
 
+def Performance : Package<"performance">;
+
----------------
I think Performance should be in the OptIn package.

================
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:194
@@ +193,3 @@
+  /// 3.  If no fields can fit, pad by rounding the current offset up to our
+  ///     lowest aligned field.  Measure and track the amount of padding added.
+  ///     Go back to 2.
----------------
It's not 100% clear what the "lowest aligned field" is.

================
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:196
@@ +195,3 @@
+  ///     Go back to 2.
+  /// 4.  Increment the current offset by the size of the chosen field
+  /// 5.  Remove the chosen field from the set of future possibilities
----------------
Punctuation is missing in 4, 5, 6.

================
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:200
@@ +199,3 @@
+  /// 7.  Add tail padding by rounding the current offset up to the structure
+  ///     alignment.  Track the amount of padding added.
+
----------------
Extra space before "Track"?


http://reviews.llvm.org/D14779