From 41f0e62d70addca57f6b2a4e734149486240be6f Mon Sep 17 00:00:00 2001 From: k Date: Thu, 15 Dec 2022 20:49:24 +0100 Subject: [PATCH 1/6] Fix @:require and false duplicate reports --- src/typing/typeloadFields.ml | 10 ++++++---- tests/misc/projects/Issue10890/Main.hx | 12 ++++++++++++ tests/misc/projects/Issue10890/build.hxml | 1 + 3 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 tests/misc/projects/Issue10890/Main.hx create mode 100644 tests/misc/projects/Issue10890/build.hxml diff --git a/src/typing/typeloadFields.ml b/src/typing/typeloadFields.ml index 25a0a963dcd..d0d23a2a1a9 100644 --- a/src/typing/typeloadFields.ml +++ b/src/typing/typeloadFields.ml @@ -1747,8 +1747,10 @@ let init_class ctx c p context_init herits fields = (match req with | None -> () | Some r -> cf.cf_kind <- Var { v_read = AccRequire (fst r, snd r); v_write = AccRequire (fst r, snd r) }); - begin match fctx.field_kind with - | FKConstructor -> + begin match req, fctx.field_kind with + | Some _, _ -> + () + | _, FKConstructor -> begin match c.cl_super with | Some ({ cl_constructor = Some ctor_sup } as c, _) when not (has_class_flag c CExtern) && has_class_field_flag ctor_sup CfFinal -> ctx.com.error "Cannot override final constructor" cf.cf_pos @@ -1765,9 +1767,9 @@ let init_class ctx c p context_init herits fields = | Some ctor -> display_error ctx.com "Duplicate constructor" p end - | FKInit -> + | _, FKInit -> () - | FKNormal -> + | _, FKNormal -> let dup = if fctx.is_static then PMap.exists cf.cf_name c.cl_fields || has_field cf.cf_name c.cl_super else PMap.exists cf.cf_name c.cl_statics in if not cctx.is_native && not (has_class_flag c CExtern) && dup then typing_error ("Same field name can't be used for both static and instance : " ^ cf.cf_name) p; if fctx.override <> None then diff --git a/tests/misc/projects/Issue10890/Main.hx b/tests/misc/projects/Issue10890/Main.hx new file mode 100644 index 00000000000..a3971d155f2 --- /dev/null +++ b/tests/misc/projects/Issue10890/Main.hx @@ -0,0 +1,12 @@ +extern class Main { + @:require(false) function test(i:Int):Void; + @:require(true) function test(s:String):Void; + @:require(true) overload function foo(i:Int):Void; + @:require(true) overload function foo(s:String):Void; +} + +class Bar { + function bar():Void {} + @:require(false) static function bar():Void {} + @:require(false) var bar:String; +} diff --git a/tests/misc/projects/Issue10890/build.hxml b/tests/misc/projects/Issue10890/build.hxml new file mode 100644 index 00000000000..c098216e78e --- /dev/null +++ b/tests/misc/projects/Issue10890/build.hxml @@ -0,0 +1 @@ +Main From 48a671e9265431ba9fb603463af6d12090afa8f8 Mon Sep 17 00:00:00 2001 From: k Date: Thu, 15 Dec 2022 20:56:42 +0100 Subject: [PATCH 2/6] Add test for (expected) failures with @:require(true) --- tests/misc/projects/Issue10890/Foo.hx | 10 ++++++++++ tests/misc/projects/Issue10890/compile-fail.hxml | 1 + .../misc/projects/Issue10890/compile-fail.hxml.stderr | 3 +++ 3 files changed, 14 insertions(+) create mode 100644 tests/misc/projects/Issue10890/Foo.hx create mode 100644 tests/misc/projects/Issue10890/compile-fail.hxml create mode 100644 tests/misc/projects/Issue10890/compile-fail.hxml.stderr diff --git a/tests/misc/projects/Issue10890/Foo.hx b/tests/misc/projects/Issue10890/Foo.hx new file mode 100644 index 00000000000..158dbf48984 --- /dev/null +++ b/tests/misc/projects/Issue10890/Foo.hx @@ -0,0 +1,10 @@ +extern class Main { + @:require(true) function test(i:Int):Void; + @:require(true) function test(s:String):Void; +} + +class Bar { + function bar():Void {} + @:require(true) static function bar():Void {} + @:require(true) var bar:String; +} diff --git a/tests/misc/projects/Issue10890/compile-fail.hxml b/tests/misc/projects/Issue10890/compile-fail.hxml new file mode 100644 index 00000000000..bc56c4d8944 --- /dev/null +++ b/tests/misc/projects/Issue10890/compile-fail.hxml @@ -0,0 +1 @@ +Foo diff --git a/tests/misc/projects/Issue10890/compile-fail.hxml.stderr b/tests/misc/projects/Issue10890/compile-fail.hxml.stderr new file mode 100644 index 00000000000..da8a65354aa --- /dev/null +++ b/tests/misc/projects/Issue10890/compile-fail.hxml.stderr @@ -0,0 +1,3 @@ +Foo.hx:8: characters 18-47 : Same field name can't be used for both static and instance : bar +Foo.hx:9: characters 22-25 : Duplicate class field declaration : Bar.bar +Foo.hx:3: characters 27-31 : Duplicate class field declaration : Main.test From 5c7e29b1bef9acaff0990838d230bfcf1016187d Mon Sep 17 00:00:00 2001 From: k Date: Thu, 15 Dec 2022 20:59:02 +0100 Subject: [PATCH 3/6] Separate static vs instance and function vs var cases --- tests/misc/projects/Issue10890/Foo.hx | 6 +++++- tests/misc/projects/Issue10890/compile-fail.hxml.stderr | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/misc/projects/Issue10890/Foo.hx b/tests/misc/projects/Issue10890/Foo.hx index 158dbf48984..170ab179b4d 100644 --- a/tests/misc/projects/Issue10890/Foo.hx +++ b/tests/misc/projects/Issue10890/Foo.hx @@ -6,5 +6,9 @@ extern class Main { class Bar { function bar():Void {} @:require(true) static function bar():Void {} - @:require(true) var bar:String; +} + +class Baz { + function baz():Void {} + @:require(true) var baz:String; } diff --git a/tests/misc/projects/Issue10890/compile-fail.hxml.stderr b/tests/misc/projects/Issue10890/compile-fail.hxml.stderr index da8a65354aa..e9889dd1c1f 100644 --- a/tests/misc/projects/Issue10890/compile-fail.hxml.stderr +++ b/tests/misc/projects/Issue10890/compile-fail.hxml.stderr @@ -1,3 +1,3 @@ +Foo.hx:13: characters 22-25 : Duplicate class field declaration : Baz.baz Foo.hx:8: characters 18-47 : Same field name can't be used for both static and instance : bar -Foo.hx:9: characters 22-25 : Duplicate class field declaration : Bar.bar Foo.hx:3: characters 27-31 : Duplicate class field declaration : Main.test From 30e10896c0d02c48b017000dd49b861b7af8d4b5 Mon Sep 17 00:00:00 2001 From: k Date: Thu, 15 Dec 2022 21:01:41 +0100 Subject: [PATCH 4/6] Cleanup --- src/typing/typeloadFields.ml | 88 +++++++++++++++++------------------- 1 file changed, 42 insertions(+), 46 deletions(-) diff --git a/src/typing/typeloadFields.ml b/src/typing/typeloadFields.ml index d0d23a2a1a9..1168d21ee05 100644 --- a/src/typing/typeloadFields.ml +++ b/src/typing/typeloadFields.ml @@ -1744,53 +1744,49 @@ let init_class ctx c p context_init herits fields = List.iter set_feature (check_if_feature cf.cf_meta); let req = check_require f.cff_meta in let req = (match req with None -> if fctx.is_static || fctx.field_kind = FKConstructor then cl_req else None | _ -> req) in - (match req with - | None -> () - | Some r -> cf.cf_kind <- Var { v_read = AccRequire (fst r, snd r); v_write = AccRequire (fst r, snd r) }); - begin match req, fctx.field_kind with - | Some _, _ -> - () - | _, FKConstructor -> - begin match c.cl_super with - | Some ({ cl_constructor = Some ctor_sup } as c, _) when not (has_class_flag c CExtern) && has_class_field_flag ctor_sup CfFinal -> - ctx.com.error "Cannot override final constructor" cf.cf_pos - | _ -> () - end; - begin match c.cl_constructor with - | None -> - c.cl_constructor <- Some cf - | Some ctor when ctx.com.config.pf_overload -> - if has_class_field_flag cf CfOverload && has_class_field_flag ctor CfOverload then - ctor.cf_overloads <- cf :: ctor.cf_overloads - else - display_error ctx.com ("If using overloaded constructors, all constructors must be declared with 'overload'") (if has_class_field_flag cf CfOverload then ctor.cf_pos else cf.cf_pos) - | Some ctor -> - display_error ctx.com "Duplicate constructor" p - end - | _, FKInit -> - () - | _, FKNormal -> - let dup = if fctx.is_static then PMap.exists cf.cf_name c.cl_fields || has_field cf.cf_name c.cl_super else PMap.exists cf.cf_name c.cl_statics in - if not cctx.is_native && not (has_class_flag c CExtern) && dup then typing_error ("Same field name can't be used for both static and instance : " ^ cf.cf_name) p; - if fctx.override <> None then - add_class_field_flag cf CfOverride; - let is_var cf = match cf.cf_kind with | Var _ -> true | _ -> false in - if PMap.mem cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) then - if has_class_field_flag cf CfOverload && not (is_var cf) then - let mainf = PMap.find cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) in - if is_var mainf then display_error ctx.com "Cannot declare a variable with same name as a method" mainf.cf_pos; - (if not (has_class_field_flag mainf CfOverload) then display_error ctx.com ("Overloaded methods must have 'overload' accessor") mainf.cf_pos); - mainf.cf_overloads <- cf :: cf.cf_overloads @ mainf.cf_overloads + match req with + | Some r -> cf.cf_kind <- Var { v_read = AccRequire (fst r, snd r); v_write = AccRequire (fst r, snd r) } + | None -> match fctx.field_kind with + | FKConstructor -> + begin match c.cl_super with + | Some ({ cl_constructor = Some ctor_sup } as c, _) when not (has_class_flag c CExtern) && has_class_field_flag ctor_sup CfFinal -> + ctx.com.error "Cannot override final constructor" cf.cf_pos + | _ -> () + end; + begin match c.cl_constructor with + | None -> + c.cl_constructor <- Some cf + | Some ctor when ctx.com.config.pf_overload -> + if has_class_field_flag cf CfOverload && has_class_field_flag ctor CfOverload then + ctor.cf_overloads <- cf :: ctor.cf_overloads + else + display_error ctx.com ("If using overloaded constructors, all constructors must be declared with 'overload'") (if has_class_field_flag cf CfOverload then ctor.cf_pos else cf.cf_pos) + | Some ctor -> + display_error ctx.com "Duplicate constructor" p + end + | FKInit -> + () + | FKNormal -> + let dup = if fctx.is_static then PMap.exists cf.cf_name c.cl_fields || has_field cf.cf_name c.cl_super else PMap.exists cf.cf_name c.cl_statics in + if not cctx.is_native && not (has_class_flag c CExtern) && dup then typing_error ("Same field name can't be used for both static and instance : " ^ cf.cf_name) p; + if fctx.override <> None then + add_class_field_flag cf CfOverride; + let is_var cf = match cf.cf_kind with | Var _ -> true | _ -> false in + if PMap.mem cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) then + if has_class_field_flag cf CfOverload && not (is_var cf) then + let mainf = PMap.find cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) in + if is_var mainf then display_error ctx.com "Cannot declare a variable with same name as a method" mainf.cf_pos; + (if not (has_class_field_flag mainf CfOverload) then display_error ctx.com ("Overloaded methods must have 'overload' accessor") mainf.cf_pos); + mainf.cf_overloads <- cf :: cf.cf_overloads @ mainf.cf_overloads + else + let type_kind,path = match c.cl_kind with + | KAbstractImpl a -> "abstract",a.a_path + | KModuleFields m -> "module",m.m_path + | _ -> "class",c.cl_path + in + display_error ctx.com ("Duplicate " ^ type_kind ^ " field declaration : " ^ s_type_path path ^ "." ^ cf.cf_name) cf.cf_name_pos else - let type_kind,path = match c.cl_kind with - | KAbstractImpl a -> "abstract",a.a_path - | KModuleFields m -> "module",m.m_path - | _ -> "class",c.cl_path - in - display_error ctx.com ("Duplicate " ^ type_kind ^ " field declaration : " ^ s_type_path path ^ "." ^ cf.cf_name) cf.cf_name_pos - else - if fctx.do_add then TClass.add_field c cf - end + if fctx.do_add then TClass.add_field c cf with Error (Custom str,p2) when p = p2 -> display_error ctx.com str p ) fields; From 0b433f1b0e0f1b41d9da173dca524eae91807506 Mon Sep 17 00:00:00 2001 From: k Date: Thu, 15 Dec 2022 21:09:43 +0100 Subject: [PATCH 5/6] Only fix @:require vs overload --- src/typing/typeloadFields.ml | 90 ++++++++++++++------------ tests/misc/projects/Issue10890/Main.hx | 12 +--- 2 files changed, 50 insertions(+), 52 deletions(-) diff --git a/src/typing/typeloadFields.ml b/src/typing/typeloadFields.ml index 1168d21ee05..28568b950a9 100644 --- a/src/typing/typeloadFields.ml +++ b/src/typing/typeloadFields.ml @@ -1744,49 +1744,55 @@ let init_class ctx c p context_init herits fields = List.iter set_feature (check_if_feature cf.cf_meta); let req = check_require f.cff_meta in let req = (match req with None -> if fctx.is_static || fctx.field_kind = FKConstructor then cl_req else None | _ -> req) in - match req with - | Some r -> cf.cf_kind <- Var { v_read = AccRequire (fst r, snd r); v_write = AccRequire (fst r, snd r) } - | None -> match fctx.field_kind with - | FKConstructor -> - begin match c.cl_super with - | Some ({ cl_constructor = Some ctor_sup } as c, _) when not (has_class_flag c CExtern) && has_class_field_flag ctor_sup CfFinal -> - ctx.com.error "Cannot override final constructor" cf.cf_pos - | _ -> () - end; - begin match c.cl_constructor with - | None -> - c.cl_constructor <- Some cf - | Some ctor when ctx.com.config.pf_overload -> - if has_class_field_flag cf CfOverload && has_class_field_flag ctor CfOverload then - ctor.cf_overloads <- cf :: ctor.cf_overloads - else - display_error ctx.com ("If using overloaded constructors, all constructors must be declared with 'overload'") (if has_class_field_flag cf CfOverload then ctor.cf_pos else cf.cf_pos) - | Some ctor -> - display_error ctx.com "Duplicate constructor" p - end - | FKInit -> - () - | FKNormal -> - let dup = if fctx.is_static then PMap.exists cf.cf_name c.cl_fields || has_field cf.cf_name c.cl_super else PMap.exists cf.cf_name c.cl_statics in - if not cctx.is_native && not (has_class_flag c CExtern) && dup then typing_error ("Same field name can't be used for both static and instance : " ^ cf.cf_name) p; - if fctx.override <> None then - add_class_field_flag cf CfOverride; - let is_var cf = match cf.cf_kind with | Var _ -> true | _ -> false in - if PMap.mem cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) then - if has_class_field_flag cf CfOverload && not (is_var cf) then - let mainf = PMap.find cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) in - if is_var mainf then display_error ctx.com "Cannot declare a variable with same name as a method" mainf.cf_pos; - (if not (has_class_field_flag mainf CfOverload) then display_error ctx.com ("Overloaded methods must have 'overload' accessor") mainf.cf_pos); - mainf.cf_overloads <- cf :: cf.cf_overloads @ mainf.cf_overloads - else - let type_kind,path = match c.cl_kind with - | KAbstractImpl a -> "abstract",a.a_path - | KModuleFields m -> "module",m.m_path - | _ -> "class",c.cl_path - in - display_error ctx.com ("Duplicate " ^ type_kind ^ " field declaration : " ^ s_type_path path ^ "." ^ cf.cf_name) cf.cf_name_pos + (match req with + | None -> () + | Some r -> cf.cf_kind <- Var { v_read = AccRequire (fst r, snd r); v_write = AccRequire (fst r, snd r) }); + begin match fctx.field_kind with + | FKConstructor -> + begin match c.cl_super with + | Some ({ cl_constructor = Some ctor_sup } as c, _) when not (has_class_flag c CExtern) && has_class_field_flag ctor_sup CfFinal -> + ctx.com.error "Cannot override final constructor" cf.cf_pos + | _ -> () + end; + begin match c.cl_constructor with + | None -> + c.cl_constructor <- Some cf + | Some ctor when ctx.com.config.pf_overload -> + if has_class_field_flag cf CfOverload && has_class_field_flag ctor CfOverload then + ctor.cf_overloads <- cf :: ctor.cf_overloads else - if fctx.do_add then TClass.add_field c cf + display_error ctx.com ("If using overloaded constructors, all constructors must be declared with 'overload'") (if has_class_field_flag cf CfOverload then ctor.cf_pos else cf.cf_pos) + | Some ctor -> + display_error ctx.com "Duplicate constructor" p + end + | FKInit -> + () + | FKNormal -> + let dup = if fctx.is_static then PMap.exists cf.cf_name c.cl_fields || has_field cf.cf_name c.cl_super else PMap.exists cf.cf_name c.cl_statics in + if not cctx.is_native && not (has_class_flag c CExtern) && dup then typing_error ("Same field name can't be used for both static and instance : " ^ cf.cf_name) p; + if fctx.override <> None then + add_class_field_flag cf CfOverride; + let is_var cf = match cf.cf_kind with + | Var {v_read = AccRequire _; v_write = AccRequire _} -> false + | Var _ -> true + | _ -> false + in + if PMap.mem cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) then + if has_class_field_flag cf CfOverload && not (is_var cf) then + let mainf = PMap.find cf.cf_name (if fctx.is_static then c.cl_statics else c.cl_fields) in + if is_var mainf then display_error ctx.com "Cannot declare a variable with same name as a method" mainf.cf_pos; + (if not (has_class_field_flag mainf CfOverload) then display_error ctx.com ("Overloaded methods must have 'overload' accessor") mainf.cf_pos); + mainf.cf_overloads <- cf :: cf.cf_overloads @ mainf.cf_overloads + else + let type_kind,path = match c.cl_kind with + | KAbstractImpl a -> "abstract",a.a_path + | KModuleFields m -> "module",m.m_path + | _ -> "class",c.cl_path + in + display_error ctx.com ("Duplicate " ^ type_kind ^ " field declaration : " ^ s_type_path path ^ "." ^ cf.cf_name) cf.cf_name_pos + else + if fctx.do_add then TClass.add_field c cf + end with Error (Custom str,p2) when p = p2 -> display_error ctx.com str p ) fields; diff --git a/tests/misc/projects/Issue10890/Main.hx b/tests/misc/projects/Issue10890/Main.hx index a3971d155f2..19a0c75ba87 100644 --- a/tests/misc/projects/Issue10890/Main.hx +++ b/tests/misc/projects/Issue10890/Main.hx @@ -1,12 +1,4 @@ extern class Main { - @:require(false) function test(i:Int):Void; - @:require(true) function test(s:String):Void; - @:require(true) overload function foo(i:Int):Void; - @:require(true) overload function foo(s:String):Void; -} - -class Bar { - function bar():Void {} - @:require(false) static function bar():Void {} - @:require(false) var bar:String; + @:require(false) overload function test(i:Int):Void; + @:require(false) overload function test(s:String):Void; } From b4f62e4110f06a46340793bcb6a6fb00217c2c85 Mon Sep 17 00:00:00 2001 From: k Date: Thu, 15 Dec 2022 21:11:23 +0100 Subject: [PATCH 6/6] Update tests --- tests/misc/projects/Issue10890/Foo.hx | 14 -------------- tests/misc/projects/Issue10890/Main.hx | 1 + tests/misc/projects/Issue10890/compile-fail.hxml | 1 - .../projects/Issue10890/compile-fail.hxml.stderr | 3 --- 4 files changed, 1 insertion(+), 18 deletions(-) delete mode 100644 tests/misc/projects/Issue10890/Foo.hx delete mode 100644 tests/misc/projects/Issue10890/compile-fail.hxml delete mode 100644 tests/misc/projects/Issue10890/compile-fail.hxml.stderr diff --git a/tests/misc/projects/Issue10890/Foo.hx b/tests/misc/projects/Issue10890/Foo.hx deleted file mode 100644 index 170ab179b4d..00000000000 --- a/tests/misc/projects/Issue10890/Foo.hx +++ /dev/null @@ -1,14 +0,0 @@ -extern class Main { - @:require(true) function test(i:Int):Void; - @:require(true) function test(s:String):Void; -} - -class Bar { - function bar():Void {} - @:require(true) static function bar():Void {} -} - -class Baz { - function baz():Void {} - @:require(true) var baz:String; -} diff --git a/tests/misc/projects/Issue10890/Main.hx b/tests/misc/projects/Issue10890/Main.hx index 19a0c75ba87..2f26d140844 100644 --- a/tests/misc/projects/Issue10890/Main.hx +++ b/tests/misc/projects/Issue10890/Main.hx @@ -1,4 +1,5 @@ extern class Main { + overload function test(b:Bool):Void; @:require(false) overload function test(i:Int):Void; @:require(false) overload function test(s:String):Void; } diff --git a/tests/misc/projects/Issue10890/compile-fail.hxml b/tests/misc/projects/Issue10890/compile-fail.hxml deleted file mode 100644 index bc56c4d8944..00000000000 --- a/tests/misc/projects/Issue10890/compile-fail.hxml +++ /dev/null @@ -1 +0,0 @@ -Foo diff --git a/tests/misc/projects/Issue10890/compile-fail.hxml.stderr b/tests/misc/projects/Issue10890/compile-fail.hxml.stderr deleted file mode 100644 index e9889dd1c1f..00000000000 --- a/tests/misc/projects/Issue10890/compile-fail.hxml.stderr +++ /dev/null @@ -1,3 +0,0 @@ -Foo.hx:13: characters 22-25 : Duplicate class field declaration : Baz.baz -Foo.hx:8: characters 18-47 : Same field name can't be used for both static and instance : bar -Foo.hx:3: characters 27-31 : Duplicate class field declaration : Main.test