Thanks! I've found and fixed a few more minor nits. Going to commit the updated patch in a few minutes after rebasing (mid-air collision with Charles's patch).
On Fri, Jun 22, 2012 at 5:00 PM, João Matos <[email protected]> wrote: >> Can you please review the attached patch? >> >> It fixes two issues with the mangler: >> http://llvm.org/bugs/show_bug.cgi?id=13176 >> http://llvm.org/bugs/show_bug.cgi?id=13177 > > Hi, was just testing the patch you posted on 13177. Nice to see a fix > for 13176 too! > > + size_t out_size_before = Out.GetNumBytesInBuffer(); > > Please change to LLVM coding standard (OutSizeBefore). > > + if (TypeBackReferences.size() < 10 && > + (Out.GetNumBytesInBuffer() - out_size_before > 1)) { > + size_t Size = TypeBackReferences.size(); > + TypeBackReferences[TypePtr] = Size; > > Can you factor the second condition? > > Something like: bool IsBackRef = (Out.GetNumBytesInBuffer() - > OutSizeBefore) > 1; > > Also the comment should probably read 1 character instead of letter. > > Otherwise looks solid and good to go! > > On Fri, Jun 22, 2012 at 10:21 PM, Timur Iskhodzhanov > <[email protected]> wrote: >> Hi Charles, João, John, >> >> Can you please review the attached patch? >> >> It fixes two issues with the mangler: >> http://llvm.org/bugs/show_bug.cgi?id=13176 >> http://llvm.org/bugs/show_bug.cgi?id=13177 >> >> It also adds a few more tests for repeated types which were mangled OK >> before, but might be worth some extra coverage. >> >> Thanks, >> Timur Iskhodzhanov, >> Google Russia > > > > -- > João Matos
fix_13176_and_13177_2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
