Wrong type when changing a number variable into a string (includes fix) #20

Closed
opened 2018-10-30 14:34:47 +00:00 by lorenzos · 1 comment
lorenzos commented 2018-10-30 14:34:47 +00:00 (Migrated from github.com)

Posted also on original repo: https://sourceforge.net/p/luaj/bugs/55/

When running the following Lua code in LuaJ, the type of the variable a doesn't change to string, but remains a number. This causes an unexpected behaviour when assigning strings that looks like numbers to a variable that was of type number. This doesn't happen when variables are first initialized to string type, like b in the example below.

a = 10 
print(type(a)) -- number
print(a) -- 10
 
b = "10.00" 
print(type(b)) -- string
print(b) -- 10.00
 
a = "10.00" 
print(type(a)) -- number, expected string!
print(a) -- 10, expected 10.00

On other Lua interpreters, the code runs as expected. I think that problem arises in the class NumberValueEntry in LuaTable.java, in particular in the method that is called when the entry needs to be changed:

public Entry set(LuaValue value) {
	LuaValue n = value.tonumber();
	if ( !n.isnil() ) {
		this.value = n.todouble();
		return this;
	} else {
		return new NormalEntry( this.key, value );
	}
}

Here you can see that if the new value can be coerced to a number, the entry is updated just by changing its double value to the new value, converted to double. Only if the new value is not number-like, a new entry with the correct type is returned. I don't get why there's the need to have a NumberValueEntry in the first place, but instead of removing it altogether, it could be fixed keeping the entry-reusing logic by checking the new value type instead of trying to coerce it to a number. The change below fixes the example above:

public Entry set(LuaValue value) {
	if (value.type() == TNUMBER) {
		LuaValue n = value.tonumber();
		if (!n.isnil()) {
			this.value = n.todouble();
			return this;
		}
	}
	return new NormalEntry( this.key, value );
}

I can make a pull request if you agree.

> Posted also on original repo: <https://sourceforge.net/p/luaj/bugs/55/> When running the following Lua code in LuaJ, the type of the variable `a` doesn't change to `string`, but remains a `number`. This causes an unexpected behaviour when assigning strings that looks like numbers to a variable that was of type `number`. This doesn't happen when variables are first initialized to `string` type, like `b` in the example below. ~~~lua a = 10 print(type(a)) -- number print(a) -- 10 b = "10.00" print(type(b)) -- string print(b) -- 10.00 a = "10.00" print(type(a)) -- number, expected string! print(a) -- 10, expected 10.00 ~~~ On other Lua interpreters, the code runs as expected. I think that problem arises in the class `NumberValueEntry` in `LuaTable.java`, in particular in [the method that is called when the entry needs to be changed](https://github.com/luaj/luaj/blob/master/src/core/org/luaj/vm2/LuaTable.java#L1248): ~~~java public Entry set(LuaValue value) { LuaValue n = value.tonumber(); if ( !n.isnil() ) { this.value = n.todouble(); return this; } else { return new NormalEntry( this.key, value ); } } ~~~ Here you can see that if the new value can be coerced to a number, the entry is updated just by changing its double value to the new value, converted to double. Only if the new value is not number-like, a new entry with the correct type is returned. I don't get why there's the need to have a `NumberValueEntry` in the first place, but instead of removing it altogether, it could be fixed keeping the entry-reusing logic by checking the new value type instead of trying to coerce it to a number. The change below fixes the example above: ~~~java public Entry set(LuaValue value) { if (value.type() == TNUMBER) { LuaValue n = value.tonumber(); if (!n.isnil()) { this.value = n.todouble(); return this; } } return new NormalEntry( this.key, value ); } ~~~ I can make a pull request if you agree.
Enyby commented 2018-10-31 13:48:54 +00:00 (Migrated from github.com)

Yes it is a bug. Make a pull request.

Yes it is a bug. Make a pull request.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: open-autonomous-connection/luaj#20