[PATCH] Fix alignment issues in LLVM.
Duncan P. N. Exon Smith
dexonsmith at apple.com
Tue Jun 16 15:32:33 PDT 2015
> On 2015-Jun-16, at 15:06, James Y Knight <jyknight at google.com> wrote:
>
>
>
> On Tue, Jun 16, 2015 at 2:04 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>
> > On 2015-Jun-16, at 09:32, Reid Kleckner <rnk at google.com> wrote:
> >
> > BTW, Duncan recently rewrote this so I'll add him.
> >
>
> LGTM once you've addressed Reid's comments, although I have some nitpicks
> below.
>
> Isn't there some logic like this in `User` too? Pete was changing it
> around recently
>
> Yes, there is. Will add an assert there too.
>
> > -
> > +// Assert objects tacked on the end of FunctionType won't be misaligned
> > +static_assert(AlignOf<FunctionType>::Alignment >= AlignOf<Type *>::Alignment,
> > + "");
>
> Please put the comment inside the assertion, so we get a decent compiler error.
>
> static_assert(..., "Expected FunctionType not to change alignment");
>
> Huh. I've always found the string rather superfluous,
It's the comment that's superfluous ;).
> given the compiler pointing to the error line of code, anyways. But, if you think it's an improvement, I can definitely go ahead and do that (and the ones in the larger patch for clang, too, of course).
It's strictly better than a comment. Typed once, documents the
code, and shows up nicely on the command-line or build log (or
other environment).
Compare, in the run-time check world:
// Some useful comment.
assert(expected);
vs:
assert(expected && "Some useful comment");
static_assert() has the `,` instead of the `&&` but it's otherwise
the same idea.
More information about the llvm-commits
mailing list