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

Support custom implicit Function Arguments like PosInfos via abstract @:fromNothing macro #6616

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/typing/type.ml
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ and tabstract = {
mutable a_this : t;
mutable a_from : t list;
mutable a_from_field : (t * tclass_field) list;
mutable a_from_nothing : tclass_field option;
mutable a_to : t list;
mutable a_to_field : (t * tclass_field) list;
mutable a_array : tclass_field list;
Expand Down Expand Up @@ -467,6 +468,7 @@ let null_abstract = {
a_this = t_dynamic;
a_from = [];
a_from_field = [];
a_from_nothing = None;
a_to = [];
a_to_field = [];
a_array = [];
Expand Down
24 changes: 14 additions & 10 deletions src/typing/typecore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -322,18 +322,22 @@ module AbstractCast = struct

let make_static_call ctx c cf a pl args t p =
if cf.cf_kind = Method MethMacro then begin
match args with
let args,f = match args with
| [e] ->
let e,f = push_this ctx e in
ctx.with_type_stack <- (WithType t) :: ctx.with_type_stack;
let e = match ctx.g.do_macro ctx MExpr c.cl_path cf.cf_name [e] p with
| Some e -> type_expr ctx e Value
| None -> type_expr ctx (EConst (Ident "null"),p) Value
in
ctx.with_type_stack <- List.tl ctx.with_type_stack;
f();
e
| _ -> assert false
[e],f
| _ ->
[],(fun () -> ())
in
ctx.with_type_stack <- (WithType t) :: ctx.with_type_stack;
let e = match ctx.g.do_macro ctx MExpr c.cl_path cf.cf_name args p with
| Some e -> type_expr ctx e Value
| None -> type_expr ctx (EConst (Ident "null"),p) Value
in
ctx.with_type_stack <- List.tl ctx.with_type_stack;
f();
unify ctx t e.etype p;
e
end else
make_static_call ctx c cf (apply_params a.a_params pl) args t p

Expand Down
3 changes: 3 additions & 0 deletions src/typing/typeload.ml
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ let module_pass_1 ctx m tdecls loadp =
a_from = [];
a_to = [];
a_from_field = [];
a_from_nothing = None;
a_to_field = [];
a_ops = [];
a_unops = [];
Expand Down Expand Up @@ -2367,6 +2368,8 @@ module ClassInitializer = struct
| _ -> error (cf.cf_name ^ ": @:from cast functions must accept exactly one argument") p
in
a.a_from_field <- (TLazy (ref (lazy_wait r)),cf) :: a.a_from_field;
| ((Meta.Custom ":fromNothing"),_,_) :: _ ->
a.a_from_nothing <- Some cf;
| (Meta.To,_,_) :: _ ->
if fctx.is_macro then error (cf.cf_name ^ ": Macro cast functions are not supported") p;
(* TODO: this doesn't seem quite right... *)
Expand Down
39 changes: 30 additions & 9 deletions src/typing/typer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,27 @@ let rec unify_call_args' ctx el args r callp inline force_inline =
| [],(_,false,_) :: _ ->
call_error (Not_enough_arguments args) callp
| [],(name,true,t) :: args ->
begin match loop [] args with
| [] when not (inline && (ctx.g.doinline || force_inline)) && not ctx.com.config.pf_pad_nulls ->
if is_pos_infos t then [mk_pos_infos t,true]
else []
| args ->
let e_def = default_value name t in
(e_def,true) :: args
begin match follow t with
| TAbstract({a_impl = Some c; a_from_nothing = Some cf} as a,tl) ->
let t = (AbstractCast.make_static_call ctx c cf a tl [] t callp,true) in
begin match args with
| [] -> [t]
| _ -> t :: (loop [] args)
end
| _ ->
begin match loop [] args with
| [] ->
if not (inline && (ctx.g.doinline || force_inline)) && not ctx.com.config.pf_pad_nulls then begin
if is_pos_infos t then [mk_pos_infos t,true]
else []
end else begin
let e_def = default_value name t in
[e_def,true]
end
| args ->
let e_def = default_value name t in
(e_def,true) :: args
end
end
| (_,p) :: _, [] ->
begin match List.rev !skipped with
Expand Down Expand Up @@ -1571,13 +1585,20 @@ let type_bind ctx (e : texpr) (args,ret) params p =
| [], [] -> given_args,missing_args,ordered_args
| [], _ -> error "Too many callback arguments" p
| (n,o,t) :: args , [] when o ->
let a = if is_pos_infos t then
let a = begin match follow t with
| TAbstract({a_impl = Some c; a_from_nothing = Some cf} as a,tl) ->
let t = AbstractCast.make_static_call ctx c cf a tl [] t p in
ordered_args @ [t]
| _ ->
if is_pos_infos t then
let infos = mk_infos ctx p [] in
ordered_args @ [type_expr ctx infos (WithType t)]
else if ctx.com.config.pf_pad_nulls then
(ordered_args @ [(mk (TConst TNull) t_dynamic p)])
else
ordered_args
ordered_args @ [(mk (TConst TNull) t_dynamic p)]
Copy link
Member Author

@frabbit frabbit Sep 27, 2017

Choose a reason for hiding this comment

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

@ncannasse does this change, actually ignoring pf_pad_null for bind, does have any effects on real code? The unit tests still pass.

Copy link
Member

Choose a reason for hiding this comment

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

well it's not ignoring pf_pad_null but is always pushing nulls whereas you're padding or not. I'm not sure I like it, but I guess we can remove extra nulls at a later stage maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, true. I actually wonder if and what implications this actually have.

Copy link
Member Author

Choose a reason for hiding this comment

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

or to be more precise, which negative implications this have.

Copy link
Member Author

Choose a reason for hiding this comment

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

and just to let you know, there are already cases where this leads to bugs: http://try-haxe.mrcdk.com/#12d04.

(*ordered_args*)
end
in
loop args [] given_args missing_args a
| (n,o,t) :: _ , (EConst(Ident "_"),p) :: _ when not ctx.com.config.pf_can_skip_non_nullable_argument && o && not (is_nullable t) ->
Expand Down
140 changes: 140 additions & 0 deletions tests/unit/src/unit/TestFromNothing.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package unit;

#if macro
import haxe.macro.Expr;
import haxe.macro.Context;
import haxe.macro.Type;
#end



@:forward
@:callable
@:arrayAccess
@:extern
private abstract Dep<T>(T) to T {

private inline function new (x:T) {
this = x;
}


@:from public static inline function fromT <T>(t:T):Dep<T> {
return new Dep(t);
}

@:fromNothing macro public static function fromNothing ():Expr {
var unexpected = () -> Context.fatalError("unexpected", Context.currentPos());
return switch Context.follow(Context.getExpectedType()) {
case TAbstract(_.toString() => "unit._TestFromNothing.Dep", [t]):
switch Context.follow(t) {
case TAbstract(_.toString() => "Int", []): macro 1;
case TAbstract(_.toString() => "unit._TestFromNothing.Int2", []): macro (2:Int2);
case TAbstract(_.toString() => "unit._TestFromNothing.Int3", []): macro (3:Int3);
case t:
unexpected();
}
case t:
unexpected();
}
}
}
#if !macro

private abstract Int2(Int) from Int to Int {}

private abstract Int3(Int) from Int to Int {}

class TestFromNothing extends Test {

function test1() {
function foo1 (?a:Dep<Int>) {
return a;
}

t(foo1() == 1);
t(foo1(2) == 2);

function foo2 (?a:Dep<Int>, ?b:Dep<Int>) {
return (a:Int)+(b:Int);
}

t(foo2() == 2);
t(foo2(2,3) == 5);
t(foo2(2) == 3);

function foo2 (?a:Dep<Int>, ?b:Dep<Int2>) {
return (a:Int) + (b:Int);
}

function foo3 (?a:Dep<Int>, ?b:Dep<Int2>, ?c:Dep<Int3>) {
return (a:Int) + (b:Int) + (c:Int);
}

t(foo2() == 3);
t(foo3() == 6);

t(foo2(7) == 9);
t(foo3(7) == 12);

t(foo2.bind()() == 3);
t(foo3.bind()() == 6);

t(foo2.bind(7)() == 9);
t(foo3.bind(7)() == 12);

t(foo2.bind(_)(7) == 9);
t(foo2.bind(_,_)(7, 3) == 10);

function foo4 <T>(x:T, plus:T->T->T, ?a:Dep<T>) {
return plus(x, a);
}

t(foo4(1, (a, b) -> a + b) == 2);

var f = foo4.bind(_, (a, b) -> a + b);
t(f(1) == 2);

t(foo4( (1:Int2), (a, b) -> (a:Int) + (b:Int) ) == 3);

var f = foo4.bind(_, (a:Int2, b:Int2) -> ((a:Int) + (b:Int):Int2) );
t(f(1) == 3);

function foo5 <T>(?x:Dep<T>, ?y:Dep<T>) {
return Std.string(x) + "-" + Std.string(y);
}

t(foo5(3) == "3-1");

function foo (?x:Int = 5, ?y:Dep<Int>) {
return Std.string(x) + "-" + Std.string(y);
}

t(foo() == "5-1");
t(foo.bind()() == "5-1");

function foo (?x:Dep<Int>, ?y:Int = 5) {
return Std.string(x) + "-" + Std.string(y);
}

t(foo() == "1-5");
t(foo.bind()() == "1-5");

function foo (?w:Int = 7, ?x:Dep<Int>, ?y:Int = 5) {
return Std.string(w) + "-" + Std.string(x) + "-" + Std.string(y);
}

t(foo() == "7-1-5");

t(foo.bind()() == "7-1-5");

function foo (?x:Int = 5, ?y:haxe.PosInfos) {
var r = Std.string(x) + "-" + (y != null);
return r;
}

t(foo.bind()() == "5-true");

}
}
#end
1 change: 1 addition & 0 deletions tests/unit/src/unit/TestMain.hx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class TestMain {
new TestGADT(),
new TestGeneric(),
new TestArrowFunctions(),
new TestFromNothing(),
#if !no_pattern_matching
new TestMatch(),
#end
Expand Down