[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