Sergey,On 15.03.21, Sergey Kaplun wrote:Igor,
Thanks for the review!
On 15.03.21, Igor Munkin wrote:
Sergey,
Thanks for the patch!
This is why it's so important to take the suite intact at first: I
didn't even notice these changes in the previous series and now I regret
a bit regarding the solution with LUAJIT_TEST_INIT: for this case it
occurs to be ugly a little. Nevertheless, I believe Francois would be
interested in this use case, so it's worth to file a bug against
lua-Harness, IMHO.
I don't think so -- usage [1] declares the way to launch the suite.
It is our problem, that we don't use it.
OK, I'll do it myself then.
<snipped>diff --git a/test/lua-Harness-tests/tap.lua b/test/lua-Harness-tests/tap.lua
index 08a99b8..5ce95e6 100644
--- a/test/lua-Harness-tests/tap.lua
+++ b/test/lua-Harness-tests/tap.lua
@@ -195,6 +195,15 @@ function todo (reason, count)
todo_reason = reason
end
+-- The last arg element is guaranteed name of tested binary.
Typo: s/last/least/.
Typo: s/guaranteed name of tested/guaranteed to be the name of the tested/.
Thanks, update the branch. See the iterative patch below.
===================================================================
diff --git a/test/lua-Harness-tests/tap.lua b/test/lua-Harness-tests/tap.lua
index 5ce95e6..e527687 100644
--- a/test/lua-Harness-tests/tap.lua
+++ b/test/lua-Harness-tests/tap.lua
@@ -195,7 +195,8 @@ function todo (reason, count)
todo_reason = reason
end
--- The last arg element is guaranteed name of tested binary.
+-- The least arg element is guaranteed to be the name
+-- of the tested binary.
function get_lua_binary_name ()
local i = 0
while arg[i] do
===================================================================
+function get_lua_binary_name ()
+ local i = 0
+ while arg[i] do
You make an excess lookup to a _G for each iteration. It's better to
pass <arg> as an argument to this function. Furthermore, it makes the
signature clearer, so user knows you're using <arg> to detect the
interpreter name.
This economy looks unnecessary -- there are only 6 calls to this
function from the whole suite. You can provide time measurement
to prove me wrong.
I disagreed that it makes signature clearer, because there is no way to
use something instead `arg` variable anyway. Please, give me an
example, when usage of custom array is preferable than usage `arg`
variable. If this function *always* uses `arg` variable only, why we
should move it outside this function?
I forgot to add "feel free to ignore" here. You can write the code youlike and I can't make you to do it another way alone. For this purposethere are two reviewers. I'll postpone my LGTM a bit for this change.Regarding "no way to use something instead arg": nobody can stop youfrom parsing shebang.
I believe the comment to the get_lua_binary_name tells exactly how the
binary is detected. Other than that - you will need to access the arg
outside the function, hence if the code is not in a trace you’ll have
to pass it on the stack, and if it is in a trace the optimizer will
CSE the arg access from within the function?
LGTM.
Sergos