Analyzer regression

[ncannasse]

I know there was a similar opened issue but I can't find it anymore.

In CastleDB, I am doing the following:

(untyped content.find("[name=color]")).spectrum("set", toColor(l.props.color)).closest(".item").css( { display : l.idToIndex == null && !l.data.match(Tiles(_) | TileInstances(_)) ? "" : "none" } );

This now gets compiled to:

var tmp1 = this.content.find("[name=color]").spectrum;
...
tmp1("set",tmp2).closest(".item").css({ display : tmp3});

But in JS this will get the wrong "this", creating a runtime error
If you want to keep this behavior, it is necessary to use a FClosure field access on .spectrum

[Simn]

That would be #4787, and goddammit who writes code like that...

[Simn]

I can't promise a fix for the RC. My naive attempts cause failing tests for #1827 and something going wrong in TestInt64.

I would like to point out that (untyped x).field causes the entire field chain to be type-less. It creates a monomorph so all subsequent field accesses are made on an open structure. This is actually worse than doing untyped x.field which at least tries to type things properly.

[ncannasse]

It it because of the untyped? Or can the analyzer loose this context in other scenarios?

[ncannasse]

Does no seem to be untyped-specific, since doing the following produces the same output, while being perfectly valid:

(content.find("[name=color]") : Dynamic).spectrum("set", toColor(l.props.color)).closest(".item").css( { display : l.idToIndex == null && !l.data.match(Tiles(_) | TileInstances(_)) ? "" : "none" } );

This is a serious regression IMHO (silently fails at runtime and hard to pinpoint)

[ncannasse]

Trying also the following, not working either:

var tmp : Dynamic = content.find("[name=color]");
tmp.spectrum("set", toColor(l.props.color)).closest(".item").css( { display : l.idToIndex == null && !l.data.match(Tiles(_) | TileInstances(_)) ? "" : "none" } );

[Simn]

It can happen with Dynamic and open structures when you expect to retain native semantics, which I find to be pretty crazy to begin with. In the general case the .spectrum field access has to be factored out because an argument could influence it. I've explained that in #4787.

[ncannasse]

@Simn this is not native semantics, this is because you're not creating a TClosure when making the .spectrum field access as you should.

[Simn]

Wait, that's the issue? Why didn't I read thatyou say so?

[ncannasse]

It's written in my first post :)

[Simn]

This might help with your actual case because we have an FAnon field access there, but it wouldn't help with Dynamic. There's no FClosure for Dynamic.

[Simn]

Actually it doesn't even help with your actual case. This is what the AST has because you make the .spectrum field access on an open structure:

{
        cf_name = spectrum;
        cf_doc = None;
        cf_type = TFun([:TInst(String, []), :TMono (Some (TInst(String, [])))], TMono (Some (TAnon)));
        cf_public = true;
        cf_meta = [];
        cf_kind = (default,null);
        cf_params = [];
        cf_expr = None;
    }

Note the cf_kind, it's not a method but a variable. We cannot create a FClosure on that because we cannot distinguish it from real function-field-variables.

[Simn]

There's nothing I can do here for the RC. I'll have to discuss this with Nicolas afterwards.

[Simn]

Note that I still don't know what to do with this. If you have any smart idea please let me know.

[ncannasse]

I don't see much of a solution here expect preventing the analyzer to create tmp vars which are TFun fields accesses, Because even if they were closures, this might lead to allocating some memory and we don't want that.

[ncannasse]

Clarifying: we can create a TFun tmp if it comes from a class var, but not from a class method, and not from an anon either.

[Simn]

Yes but the problem is that due to your untyped we don't know if it's a var or a method. The cf_kind is going to be Var by default.

[ncannasse]

Yes, you have to look at the object as well, if it's a TInst then you are allowed to create a tmp Var, if not then you should not do it.

[Simn]

That still doesn't sound quite right to me, but I'll take a look.

[Simn]

I have applied the fix as you suggested. I still don't think it's foolproof like that, but it probably requires some maliciousness to cause problems with it.