[llvm] r295765 - Don't modify archive members unless really needed.

[Rafael Avila de Espindola via llvm-commits]

Oops. I forgot to put the unreachable after the switch. I will fix that when I get back from lunch.

Cheers,
Rafael

On February 22, 2017 12:53:10 PM EST, Rui Ueyama <ruiu at google.com> wrote:
>Huh, that's right. Maybe it was an MSVC thing that it cannot defer that
>the
>end of function is unreachable.
>
>On Wed, Feb 22, 2017 at 6:17 AM, Rafael Avila de Espindola <
>rafael.espindola at gmail.com> wrote:
>
>> Rui Ueyama <ruiu at google.com> writes:
>>
>> > On Tue, Feb 21, 2017 at 12:40 PM, Rafael Espindola via llvm-commits
><
>> > llvm-commits at lists.llvm.org> wrote:
>> >
>> >> Author: rafael
>> >> Date: Tue Feb 21 14:40:54 2017
>> >> New Revision: 295765
>> >>
>> >> URL: http://llvm.org/viewvc/llvm-project?rev=295765&view=rev
>> >> Log:
>> >> Don't modify archive members unless really needed.
>> >>
>> >> For whatever reason ld64 requires that member headers (not the
>member
>> >> themselves) should be aligned. The only way to do that is to edit
>the
>> >> previous member so that it ends at an aligned boundary.
>> >>
>> >> Since modifying data put in an archive is an undesirable property,
>> >> llvm-ar should only do it when it is absolutely necessary.
>> >>
>> >> Added:
>> >>     llvm/trunk/test/Object/archive-pad.test
>> >> Modified:
>> >>     llvm/trunk/include/llvm/Object/Archive.h
>> >>     llvm/trunk/lib/Object/ArchiveWriter.cpp
>> >>     llvm/trunk/test/Object/archive-extract.test
>> >>     llvm/trunk/test/Object/archive-format.test
>> >>     llvm/trunk/tools/llvm-ar/llvm-ar.cpp
>> >>
>> >> Modified: llvm/trunk/include/llvm/Object/Archive.h
>> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
>> >> llvm/Object/Archive.h?rev=295765&r1=295764&r2=295765&view=diff
>> >> ============================================================
>> >> ==================
>> >> --- llvm/trunk/include/llvm/Object/Archive.h (original)
>> >> +++ llvm/trunk/include/llvm/Object/Archive.h Tue Feb 21 14:40:54
>2017
>> >> @@ -212,6 +212,7 @@ public:
>> >>      K_GNU,
>> >>      K_MIPS64,
>> >>      K_BSD,
>> >> +    K_DARWIN,
>> >>      K_DARWIN64,
>> >>      K_COFF
>> >>    };
>> >>
>> >> Modified: llvm/trunk/lib/Object/ArchiveWriter.cpp
>> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/
>> >> ArchiveWriter.cpp?rev=295765&r1=295764&r2=295765&view=diff
>> >> ============================================================
>> >> ==================
>> >> --- llvm/trunk/lib/Object/ArchiveWriter.cpp (original)
>> >> +++ llvm/trunk/lib/Object/ArchiveWriter.cpp Tue Feb 21 14:40:54
>2017
>> >> @@ -122,12 +122,26 @@ static void printWithSpacePadding(raw_fd
>> >>    }
>> >>  }
>> >>
>> >> +static bool isBSDLike(object::Archive::Kind Kind) {
>> >> +  switch (Kind) {
>> >> +  case object::Archive::K_GNU:
>> >> +    return false;
>> >> +  case object::Archive::K_BSD:
>> >> +  case object::Archive::K_DARWIN:
>> >> +    return true;
>> >> +  case object::Archive::K_MIPS64:
>> >> +  case object::Archive::K_DARWIN64:
>> >> +  case object::Archive::K_COFF:
>> >> +    llvm_unreachable("not supported for writting");
>> >> +  }
>> >> +}
>> >>
>> >
>> > This `switch` doesn't cover all Kind enums, which is bad because
>this
>> > function should return a value.
>>
>> I am pretty sure it covers all the enum values. Which one do you
>think
>> is missing?
>>
>> Cheers,
>> Rafael
>>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170222/29e9cbb9/attachment.html>