Re: [Tarantool-patches] [PATCH luajit 2/2] OSX/iOS/ARM64: Fix bytecode embedding in Mach-O object file.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Max,

changes force-pushed.

On 3/19/24 19:15, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the fixes!
> See my answer for the last unresolved comment below.
> On Tue, Mar 19, 2024 at 11:22:20AM +0300, Sergey Bronnikov wrote:
>> Max,
>>
>> thanks for review. See my comments below.
>>
>> Changes force-pushed.
>>
>> On 3/18/24 16:44, Maxim Kokryashkin wrote:
> <snipped>
>>>> diff --git a/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua b/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua
>>>> index 0519e134..0a11f163 100644
>>>> --- a/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua
>>>> +++ b/test/tarantool-tests/lj-865-fix_generation_of_mach-o_object_files.test.lua
>>>> @@ -7,7 +7,7 @@ local test = tap.test('lj-865-fix_generation_of_mach-o_object_files'):skipcond({
>>>>      ['Test uses exotic type of loaders (see #9671)'] = _TARANTOOL,
>>>>    })
>>>>
>>>> -test:plan(4)
>>>> +test:plan(8)
>>>>
>>>>    -- Test creates an object file in Mach-O format with LuaJIT bytecode
>>>>    -- and checks validness of the object file fields.
> Typo: s/validness/the validity/
Fixed.
>>>> @@ -267,5 +267,7 @@ end
>>>>
>>>>    -- ARM
>>>>    build_and_check_mach_o(false)
>>>> +-- ARM64
>>>> +build_and_check_mach_o(true)
>>> These `true/false` should be explained as platform toggle.
>>> An even better solution would be to pass the platform name
>>> explicitly.
>> Why comment above is not sufficient?
> Comment is sufficient, sure, but why should you opt for a comment, when
> you can write self-explanatory code instead?

Fixed.

'arm' or 'arm64' are passed to a test func.

I believe this is clear enough for reader.

>>>>    test:done(true)
>>>> --
>>>> 2.34.1
>>>>