Tarantool development patches archive
 help / color / mirror / Atom feed
From: Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 3/3] FFI/ARM64/OSX: Fix vararg call handling.
Date: Thu, 13 May 2021 13:44:18 +0300
Message-ID: <20210513104418.GG3944@tarantool.org> (raw)
In-Reply-To: <A3603882-10EE-4229-9A80-AED8A67D0443@tarantool.org>

Sergos,

On 13.05.21, Sergey Ostanevich wrote:
> Well, this doesn't help me with
> 
> > Since all LJ_TARGET_IOS usage is done with LJ_TARGET_ARM64 define being
> > set, we can simply replace all occurrences with LJ_TARGET_OSX.

OK, as a result of offline discussion I've finally got it (hope you
too). Everything written above relates only to FFI sources that are
touched within this changeset (consider FFI prefix in commit subject).

> 
> alongside with
> 
> > src/lj_prng.c:112:#if LJ_TARGET_OSX && !LJ_TARGET_IOS

Strictly saying, this commit[1] has not been backported yet.

> 
> because if we apply first then the second will evaluate into
> 
> src/lj_prng.c:112:#if LJ_TARGET_OSX && !LJ_TARGET_OSX
> 
> which is always false?
> 
> Also there are still operable 32bit apps, including games, so iOS
> still supports 32bits - can this change cause problems? I believe Mike
> doesn’t care too much, so it can easily slip through.

As we discussed, 32-bit platforms are not touched in this commit, so
nothing criminal.

Anyway, I see this is unclear to you, but everything is OK for Sergey.
Do I need to reword this part of commit message or even drop it?

> 
> Sergos
> 
> 
> 
> > On 13 May 2021, at 00:59, Igor Munkin <imun@tarantool.org> wrote:
> > 
> > Sergos,
> > 
> > On 12.05.21, Sergey Ostanevich wrote:
> >> Hi!
> >> 
> >> I can’t get this then
> >> 
> >>  src/lj_prng.c:112:#if LJ_TARGET_OSX && !LJ_TARGET_IOS
> > 
> > There is a verbose comment nearby[1] and the corresponding issue[2].
> > 
> >> 
> >> How’s that survived in 2.1 branch?
> >> 
> >> regards,
> >> Sergos
> >> 
> >> 
> >>> On 11 May 2021, at 14:31, Igor Munkin <imun@tarantool.org> wrote:
> >>> 
> >>> Sergey,
> >>> 
> >>> Thanks for your review!
> >>> 
> >>> On 11.05.21, Sergey Kaplun wrote:
> >>>> Hi, Igor!
> >>>> 
> >>>> Thanks for the patch!
> >>>> LGTM!
> >>> 
> >>> Added your tag:
> >>> | Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>
> >>> 
> >>> Also mentioned the issue[1]:
> >>> | Resolves tarantool/tarantool#6066
> >>> 
> >>>> 
> >>>> -- 
> >>>> Best regards,
> >>>> Sergey Kaplun
> >>> 
> >>> [1]: https://github.com/tarantool/tarantool/issues/6066
> >>> 
> >>> -- 
> >>> Best regards,
> >>> IM
> >> 
> > 
> > [1]: https://github.com/LuaJIT/LuaJIT/blob/v2.1/src/lj_prng.c#L113-L119
> > [2]: https://github.com/LuaJIT/LuaJIT/issues/668
> > 
> > -- 
> > Best regards,
> > IM
> 

[1]: https://github.com/LuaJIT/LuaJIT/commit/7877369

-- 
Best regards,
IM

  reply	other threads:[~2021-05-13 11:05 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10 22:09 [Tarantool-patches] [PATCH luajit 0/3] Basic fixes for LuaJIT on ARM64 Macs Igor Munkin via Tarantool-patches
2021-05-10 22:09 ` [Tarantool-patches] [PATCH luajit 1/3] build: pass sysroot to MacOS SDK Igor Munkin via Tarantool-patches
2021-05-11  9:49   ` Sergey Kaplun via Tarantool-patches
2021-05-12 21:55     ` Igor Munkin via Tarantool-patches
2021-05-14 16:07       ` Sergey Kaplun via Tarantool-patches
2021-05-17 17:21         ` Igor Munkin via Tarantool-patches
2021-05-18  5:50           ` Sergey Kaplun via Tarantool-patches
2021-05-18 18:47             ` Igor Munkin via Tarantool-patches
2021-05-19 11:38               ` Igor Munkin via Tarantool-patches
2021-05-19 12:40                 ` Sergey Ostanevich via Tarantool-patches
2021-05-19 13:23                   ` Igor Munkin via Tarantool-patches
2021-05-19 16:06                     ` Sergey Kaplun via Tarantool-patches
2021-05-10 22:09 ` [Tarantool-patches] [PATCH luajit 2/3] OSX/iOS: Handle iOS simulator and ARM64 Macs Igor Munkin via Tarantool-patches
2021-05-11 11:02   ` Sergey Kaplun via Tarantool-patches
2021-05-11 11:03     ` Igor Munkin via Tarantool-patches
2021-05-14 11:36       ` Sergey Ostanevich via Tarantool-patches
2021-05-14 11:27         ` Igor Munkin via Tarantool-patches
2021-05-10 22:09 ` [Tarantool-patches] [PATCH luajit 3/3] FFI/ARM64/OSX: Fix vararg call handling Igor Munkin via Tarantool-patches
2021-05-11 11:07   ` Sergey Kaplun via Tarantool-patches
2021-05-11 11:31     ` Igor Munkin via Tarantool-patches
2021-05-12 16:11       ` Sergey Ostanevich via Tarantool-patches
2021-05-12 21:59         ` Igor Munkin via Tarantool-patches
2021-05-13  9:50           ` Sergey Ostanevich via Tarantool-patches
2021-05-13 10:44             ` Igor Munkin via Tarantool-patches [this message]
2021-05-14 10:10               ` Sergey Ostanevich via Tarantool-patches
2021-05-14 10:31                 ` Igor Munkin via Tarantool-patches
2021-05-19 15:38 ` [Tarantool-patches] [PATCH luajit 0/3] Basic fixes for LuaJIT on ARM64 Macs Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210513104418.GG3944@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=sergos@tarantool.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git