HL ttype infinity

[Simn]

There are several problems with the HL generator related to its ttype usage. I'm fixing an obvious one related to infinite recursion here, but there's at least one other: if ttype is used as key to PMap and is recursive, that won't work.

This means we need a different data structure for this, but I'm not sure what's best to use, so let's ask the robot.

[Simn]

@copilot Please take a look and see what data structure to use here!

Related issues for tests:

#9662
#12239
#12256

[Copilot]

@Simn I've opened a new pull request, #12683, to work on those changes. Once the pull request is ready, I'll request review from you.

[Simn]

Interesting, my fix already breaks something too...

Edit: never mind, dumb copy & paste mistake.

[yuxiaomao]

I checked the code, and I can't find a case where it fail yet, so that's probably a good sign.

However hlcode.txt gives me different result for the following code

typedef TreeA = {
	var ?node : TreeA;
}

function main() {
	var a : TreeA = { node : null };
	trace(a);
}

Before copilote changes, i.e. on commit 992f68c:

		r0 virtual(node:virtual(node:...))
		r1 virtual(node:...)
		.6     @0 new 0
		.6     @1 null 1
		.6     @2 setfield 0[0],1

After copilote changes:

		r0 dynobj
		r1 virtual(node:...)
		r2 virtual(node:virtual(node:...))
		.6     @0 new 0
		.6     @1 null 1
		.6     @2 dynset 0[@49],1
		.6     @3 tovirtual 2,0
		.6     @4 tovirtual 1,2

Copilote changes makes a HDynobj and 2 OToVirtual calls (it'll reduce to 1 call if node isn't optional), and it seems less efficient compared to previous implementation.

[Simn]

My initial guess is that this is the PMap.find not picking up the same anon because the field optionality differs. This is exactly the case I was worried about with two different virtual_proto instances that are supposed to actual be "equal" as far as the run-time is concerned.

[Simn]

This also means strict mode anon identification isn't going to work because it won't consider these types equal either. This is also related to #11474.

[Simn]

@copilot Please check again if ctx.anons_cache can be replaced by tanon_identification. See #12683 for some previous data on that. We have made changes in #12717 to allow for a more flexible configuration, which should allow this to work now. Also check @yuxiaomao's comment above regarding output changes.

[Copilot]

@Simn I've opened a new pull request, #12718, to work on those changes. Once the pull request is ready, I'll request review from you.

[Simn]

@yuxiaomao Please see if it works for you now!

[yuxiaomao]

Observation on shiro's code base:

0. This PR works better on previous sample, on the type of a: r0 virtual(node:...) instead of r0 virtual(node:virtual(node:...))
1. Some native function's "fid" changed (theirs order in natives does not change), I don't know why but code still works so probably fine
2. This PR works better than development in some aspects, e.g.

function main() {
	var a = new App();
	a.mainLoop();
}

interface IDrawable {
	public function render( engine : Engine ) : Void;
}

class App implements IDrawable {
	public function new() {
	}
	public function mainLoop() {
		var engine = new Engine();
		engine.render(this);
	}
	public function render( e : Engine ) {
	}
}

class Engine {
	public function new() {
	}
	public function render( obj : { function render( engine : Engine ) : Void; } ) {
	}
}

Development has a strange additional field get/set

		.16    @2 field 3,0[0]
		.16    @3 jnotnull 3,2
		.16    @4 tovirtual 3,0
		.16    @5 setfield 0[0],3
		.16    @6 call 2, Engine.render(1,3)
		.17    @7 ret 2

PR

		.16    @4 tovirtual 3,0
		.16    @6 call 2, Engine.render(1,3)
		.17    @7 ret 2

4. However, sometimes this PR made two tovirtual sequence with same definition of register, which is a regression. E.g.

typedef WheelGroup = {
	?choices : Array<String>,
	?neutralChoice: String,
}

function main() {
	var icons : Array<String> = [];
	new ChoiceWheel({
		choices: icons,
	});
}

class ChoiceWheel {
	public function new( group : WheelGroup ) {
	}
}

Development

		r7 dynobj
		r8 virtual(choices:hl.types.ArrayObj,neutralChoice:String)
		.9     @8 tovirtual 8,7
		.8     @9 call 5, ChoiceWheel.new(6,8)
		.11    @A ret 5

PR

		r7 dynobj
		r8 virtual(choices:hl.types.ArrayObj,neutralChoice:String)
		r9 virtual(choices:hl.types.ArrayObj,neutralChoice:String)
		.9     @8 tovirtual 8,7
		.9     @9 tovirtual 9,8
		.8     @A call 5, ChoiceWheel.new(6,9)
		.11    @B ret 5

5. Also sometimes virtual can't be detected properly by this PR

enum EKind {
	Empty;
}

typedef ApplicationDesc = {
	@:optional final target: EKind; //@:optional and final each remove part of strange thing
}

function registerAffixDesc(): ApplicationDesc {
	return { target : Empty };
}

function main() {
	registerAffixDesc();
}

Development (expected)

		r0 virtual(target:enum(EKind))
		r1 enum(EKind)
		.10    @0 new 0
		.10    @1 global 1, 7
		.10    @2 setfield 0[0],1
		.10    @3 ret 0

PR (dynobj instead of virtual, two tovirtual calls)

		r0 dynobj
		r1 enum(EKind)
		r2 virtual(target:enum(EKind))
		r3 virtual(target:enum(EKind))
		.10    @0 new 0
		.10    @1 global 1, 7
		.10    @2 dynset 0[@100],1
		.10    @3 tovirtual 2,0
		.10    @4 tovirtual 3,2
		.10    @5 ret 3