[PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

[Ben Craig via cfe-commits]

bcraig added a comment.

Note: In the future, try to provide more context lines in the patch.  http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

I'm mostly fine with the algorithmic changes.  I'm not thrilled with the formatting of the error message, but I'm not sure if much can be done about it without causing tons of work.

Here's my wishlist items...

1. List each field on an individual line, preferably with the type mentioned as well.
2. Get the field ordering information into a note or fixit.
3. Provide a more stable ("stable" as in stable sort) field ordering.  I've got an inline comment describing what I mean with that.

I'm not sure if my first two items are possible without large overhauls.  The third isn't a big deal either.

So to recap, I'm strongly in favor of the idea.  I'm weakly in favor of the implementation.  I'll give you a shot to polish things a bit if you have the inclination.  If you don't, then I won't let my idea of a perfect (and expensive) implementation stand in the way of this good implementation.


https://reviews.llvm.org/D23387