<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Feb 17, 2015 at 1:42 AM, Renato Golin <span dir="ltr"><<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On 17 February 2015 at 00:38, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>> wrote:<br>
>   std::size_t Size = sizeof(DeclRefExpr);<br>
>   if (...)<br>
>     Size += sizeof(...);<br>
>   ...<br>
>   void *Mem = Context.Allocate(Size, llvm::alignOf<DeclRefExpr>());<br>
<br>
</span>This sounds wrong. You should have something like:<br>
<span class=""><br>
   std::size_t Size = sizeof(DeclRefExpr);<br>
</span>   std::size_t Align = llvm::alignOf<DeclRefExpr>();<br>
<span class="">   if (...) {<br>
     Size += sizeof(...);<br>
</span>     Align = std::max(Align, llvm::alignOf<...>());<br>
   }<br>
   ...<br>
   void *Mem = Context.Allocate(Size, Align);</blockquote><div><br></div><div>This isn't enough either. You need to update the accessors to account for alignment. They currently look like this:</div><div><br></div><div><div>  NamedDecl *&getInternalFoundDecl() {</div><div>    assert(hasFoundDecl());</div><div>    if (hasQualifier())</div><div>      return *reinterpret_cast<NamedDecl **>(&getInternalQualifierLoc() + 1);</div><div>    return *reinterpret_cast<NamedDecl **>(this + 1);</div><div>  }</div></div><div><br></div><div>I do *not* want to audit Clang for the 'reinterpret_cast<Foo*>(this + 1)' pattern and add extra alignment code. We should just make sure that anything allocated this way requires only pointer-size alignment.</div><div><br></div><div>I put this on <a href="http://llvm.org/pr22727">http://llvm.org/pr22727</a>.</div></div></div></div>