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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
