Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revive boolean type #227

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

snoopcatt
Copy link

Hello!
I'm here to resurrect the boolean type.
Looks like it was dead a long time ago and buried in source code.

As for current Ravi version, there is zombie boolean type. Look:

local function bool(b: boolean)
        print(b, ravitype(b))
end

bool(true)

ravi: /home/ann/bool.ravi:0: type mismatch: expected boolean

Literally, boolean type exists, but it just does not work -- nor true nor false are "not boolean".


After some necromancy, boolean shows signs of life.

local function bool(b: boolean)
	print(b, ravitype(b))
end

bool(true)
bool('world')

true boolean
src/ravi: /home/ann/bool.ravi:0: boolean expected

That works too:

local b: boolean = true
b = 123

src/ravi: b.ravi:4: Invalid assignment: boolean expected near <eof>

But I could forget to add it somewhere, please, double check it.

@XmiliaH
Copy link
Contributor

XmiliaH commented May 20, 2021

It should be discussed if

local x:boolean
x = (function() return 1 end)()
print(x)

should error, since 1 is no boolean or convert 1 to a boolean. As

local x:number
x = (function() return "1" end)()
print(x)

does for strings.

@snoopcatt
Copy link
Author

I think it shouldn't.
Because in Lua representation, anything that is not nil or false considered as true.

It means that both 0 (number) and "0" (string) are true (boolean) in Lua world.
Besides 0 and "1" we have also "true" and "false" strings.


But we may discuss implementation of explicit cast to @boolean(var).
It should return:

  • true if var == 1 (number) or "1" (string) or "true" (string)
  • false if var == 0 (number) or "0" (string) or "false" (string)
  • nil otherwise
function toboolean(v)
  if (v == 1 or v == '1' or v == 'true' ) then return true elseif (v == 0 or v == '0' or v == 'false') then return false end
end

@snoopcatt
Copy link
Author

snoopcatt commented May 20, 2021

It should be discussed if

I implemented it in second commit.
Seems to be working.

function test(i: integer) -- boolean
	return @boolean(i)
end

local x:boolean
x = (function() return 1 end)()
print(x)

print(test(1))

true
true

But now I don't know, good it or bad, that string/number converting to boolean sometimes lol

** On successful conversion returns 1 and stores value in (*b).
** On failure, returns 0.
*/
int luaV_toboolean (const TValue *obj, int *b) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't comply with Lua semantics - string and numbers are true.

@dibyendumajumdar
Copy link
Owner

dibyendumajumdar commented May 22, 2021

In general - boolean is a dubious type in Lua. It was added just to allow something like NIL value in a table. Roberto has said many times he regrets adding this type. I want to understand what is the benefit of adding this type? Is there a good use case?

@@ -2589,6 +2616,13 @@ int luaV_execute (lua_State *L) {
luaG_runerror(L, "string expected");
vmbreak;
}
vmcase(OP_RAVI_TOBOOLEAN) {
int b;
if (RAVI_LIKELY(luaV_toboolean(ra, &b))) { setbvalue(ra, b); }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not convinced we should conversion.

(void)pc;
emit_reg(fn, "ra", A);
membuff_add_string(&fn->body, "int bool = 0;\n");
membuff_add_string(&fn->body, "if (luaV_toboolean(ra, &bool)) { setbvalue(ra, bool); }\n");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, not convinced conversion is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants