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

Dynamic vs. String #9586

Closed
wants to merge 2 commits into from
Closed

Dynamic vs. String #9586

wants to merge 2 commits into from

Conversation

Simn
Copy link
Member

@Simn Simn commented Jun 15, 2020

For #9585. Currently fails on HL, PHP and python.

  • HL
unit.issues.Issue9585
  test: ERROR E
    Can't cast f64 to String
Called from unit.issues.Issue9585.test (unit/issues/Issue9585.hx line 27)
Called from unit.issues.Issue9585.~__initializeUtest__.0 (unit/issues/Issue9585.hx line 26)
  • PHP
unit.issues.Issue9585
  test: ERROR E
    ErrorException: Object of class class@anonymous could not be converted to string in /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/unit/issues/Issue9585.php:52
Stack trace:
#0 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/unit/issues/Issue9585.php(52): php\Boot::php\{closure}(4096, 'Object of class...', '/home/runner/wo...', 52, Array)
#1 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/unit/issues/Issue9585.php(35): unit\issues\Issue9585->test()
#2 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/php/_Boot/HxAnon.php(31): unit\issues\Issue9585->unit\issues\{closure}()
#3 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/utest/ITestHandler.php(212): php\_Boot\HxAnon->__call('execute', Array)
#4 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/utest/ITestHandler.php(99): utest\ITestHandler->runTest()
#5 [internal function]: utest\ITestHandler->checkSetup()
#6 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/php/_Boot/HxClosure.php(59): call_user_func_array(Array, Array)
#7 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/utest/Async.php(155): php\_Boot\HxClosure->__invoke()
#8 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/utest/ITestHandler.php(180): utest\Async->then(Object(php\_Boot\HxClosure))
#9 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/utest/ITestHandler.php(158): utest\ITestHandler->runSetup()
#10 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/utest/Runner.php(413): utest\ITestHandler->execute()
#11 /home/runner/work/haxe/haxe/tests/unit/bin/php/lib/utest/_Runner/ITestRunner.php(162): utest\Runner->runFixture(Object(utest\TestFixture))
  • Python
unit.issues.Issue9585
  test: ERROR E
    TypeError('must be str, not float',)
Called from <unknown>.test (bin/unit.py line 89546)
Called from <unknown>._hx_local_0 (bin/unit.py line 89556)
Called from <unknown>.runTest (bin/unit.py line 104545)

@Simn Simn added platform-php Everything related to PHP platform-python Everything related to Python platform-hl Everything related to HashLink labels Jun 15, 2020
@RealyUniqueName
Copy link
Member

Do you really want to specify that any value in any runtime can be converted to a string?

@Simn
Copy link
Member Author

Simn commented Jun 15, 2020

We already specify that this works in the context of string concatenation.

@RealyUniqueName
Copy link
Member

But we do that by inserting Std.string for non-stringifiable values. So, now we have to generate Std.string(v) even if v is a String?

@Simn
Copy link
Member Author

Simn commented Jun 16, 2020

My main point is that I think these two traces should behave the same way:

function getDynamic():Dynamic {
	return 12;
}

function main() {
	trace("Value: " + getDynamic());

	var value = getDynamic();
	trace("Value: " + value);
}

Ultimately this is an issue with Dynamic not being bound to monomorphs, but I don't think this is something we'll be able to fix anytime soon.

@RealyUniqueName
Copy link
Member

I think current behavior is correct. It's specified that monomorph is not bound to Dynamic and it's specified that monomorph is bound to String on concatenation. It's not easy to understand this behavior for a user, but I think it's a rare case and generating Std.string calls everywhere would be much worse.

@RealyUniqueName
Copy link
Member

Otherwise we'll have to do something with all the similar cases. E.g. #9524

@Simn
Copy link
Member Author

Simn commented Jun 16, 2020

What do you mean "everywhere"? This is specifically about assigning Dynamic to String.

@RealyUniqueName
Copy link
Member

I mean in general it's impossible to detect if a value came from Dynamic.

function print(s:String) {
  trace('Message: ' + s);
}

var v = getDynamic();
print(v);

have to be generated as

function print(s:String) {
  trace('Message: ' + Std.string(s)); //Std.string on every concatenation
}

var v = getDynamic();
print(v);

@Simn
Copy link
Member Author

Simn commented Jun 16, 2020

The conversion would be done var v = /* here */ getDynamic() because that's the Dynamic to String assignment. That's what e.g. hxcpp already does:

void Main_Fields__obj::main(){
            	HX_STACKFRAME(&_hx_pos_195033e4c87195a0_9_main)
HXLINE(  10)		::String v = ( (::String)(::_Main::Main_Fields__obj::getDynamic()) );
HXLINE(  11)		::_Main::Main_Fields__obj::print(v);
            	}

And Java:

	public static void main()
	{
		//line 10 "C:\\git\\haxerepro\\source\\Main.hx"
		java.lang.String v = haxe.lang.Runtime.toString(_Main.Main_Fields_.getDynamic());
		//line 11 "C:\\git\\haxerepro\\source\\Main.hx"
		_Main.Main_Fields_.print(v);
	}

And C#:

		public static void main() {
			#line 10 "C:\\git\\haxerepro\\source\\Main.hx"
			string v = global::haxe.lang.Runtime.toString(global::_Main.Main_Fields_.getDynamic());
			global::_Main.Main_Fields_.print(v);
		}

And I guess Flash too because it passes these tests.

@RealyUniqueName
Copy link
Member

That's better. But it still feels wrong.
We keep adding guarantees for Dynamic while telling people Dynamic provides no guarantees and dreaming to get rid of it :)

Quote from the manual:

It is very easy to come up with examples where the usage of Dynamic can cause problems at runtime.

And yet we try to fix those problems.

@Simn
Copy link
Member Author

Simn commented Jun 16, 2020

Ironically, part of the problem in #9585 is that he doesn't use Dynamic (for the local variable types).

@RealyUniqueName
Copy link
Member

Why don't we generate Std.string in the "core" of the compiler instead of implementing the same logic in every generator?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-hl Everything related to HashLink platform-php Everything related to PHP platform-python Everything related to Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants