tekknolagi added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:167-171
}
+ // How do you reorder fields if you haven't got any?
+ else if (RD->field_empty())
+ return true;
+
----------------
bcraig wrote:
> I think this should just be an if, and not an else if.
Changing it to a normal if mucks with the fall-through if pattern: in the old
behavior, any empty struct would be skipped. In this new behavior, it shouldn't
be the same -- it should only be the empty structs that don't have any base
classes, etc.
================
Comment at: test/Analysis/padding_inherit.cpp:2-4
+
+// A class that has no fields and one base class should visit that base class
+// instead.
----------------
bcraig wrote:
> Evil test case to add...
>
> ```
> struct Empty {};
> struct DoubleEmpty : Empty {
> Empty e;
> };
> ```
>
> I don't think that should warn, as you can't reduce padding by rearranging
> element.
>
> I guess your new code shouldn't catch that at all anyway, as you are testing
> in the other direction.... when field-less inherits from field-ed.
>
I'll add that test case -- certainly something to consider when somebody
tackles the harder field/inheritance problem.
Repository:
rC Clang
https://reviews.llvm.org/D53206
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
