Skip to content

Commit

Permalink
Clean up warning handling and add optional WUnsafeEnumEquality (#11826)
Browse files Browse the repository at this point in the history
* clean up warning handling and add optional WUnsafeEnumEquality

* wah
  • Loading branch information
Simn authored and kLabz committed Mar 6, 2025
1 parent c3c019d commit 08243f6
Show file tree
Hide file tree
Showing 13 changed files with 73 additions and 17 deletions.
9 changes: 7 additions & 2 deletions src-json/warning.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,11 @@
"name": "WConstructorInliningCancelled",
"doc": "Constructor call could not be inlined because a field is uninitialized",
"parent": "WTyper"
},
{
"name": "WUnsafeEnumEquality",
"doc": "Equality operations on enums with parameters might not work as expected",
"parent": "WTyper",
"enabled": false
}

]
]
15 changes: 15 additions & 0 deletions src-prebuild/prebuild.ml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ type parsed_warning = {
w_doc : string;
w_parent : string option;
w_generic : bool;
w_enabled : bool;
}

let as_string = function
Expand Down Expand Up @@ -131,6 +132,7 @@ let parse_warning json =
w_doc = get_field "doc" as_string fields;
w_parent = get_optional_field2 "parent" as_string fields;
w_generic = get_optional_field "generic" as_bool false fields;
w_enabled = get_optional_field "enabled" as_bool true fields;
}

let parse_file_array path map =
Expand Down Expand Up @@ -244,6 +246,15 @@ let gen_warning_obj warnings =
) warnings in
String.concat "\n" warning_str

let gen_disabled_warnings warnings =
let warning_str = ExtList.List.filter_map (fun w ->
if w.w_enabled then
None
else
Some w.w_name
) warnings in
String.concat ";" warning_str

let autogen_header = "(* This file is auto-generated using prebuild from files in src-json *)
(* Do not edit manually! *)
"
Expand Down Expand Up @@ -344,6 +355,10 @@ match Array.to_list (Sys.argv) with
print_endline "";
print_endline "let warning_obj = function";
print_endline (gen_warning_obj warnings);
print_endline ";;";
print_endline "let disabled_warnings = [";
print_endline (gen_disabled_warnings warnings);
print_endline "];;";
print_endline "";
print_endline "let from_string = function";
print_endline (gen_warning_parse warnings);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/compiler.ml
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ module Setup = struct
Common.define_value com Define.Dce "std";
com.info <- (fun ?(depth=0) msg p -> message ctx (make_compiler_message msg p depth DKCompilerMessage Information));
com.warning <- (fun ?(depth=0) w options msg p ->
match Warning.get_mode w (com.warning_options @ options) with
match Warning.get_mode w (options @ com.warning_options) with
| WMEnable ->
let wobj = Warning.warning_obj w in
let msg = if wobj.w_generic then
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/displayProcessing.ml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ let process_display_configuration ctx =
add_diagnostics_message ?depth com (located s p) DKCompilerMessage Information
);
com.warning <- (fun ?depth w options s p ->
match Warning.get_mode w (com.warning_options @ options) with
match Warning.get_mode w (options @ com.warning_options) with
| WMEnable ->
add_diagnostics_message ?depth com (located s p) DKCompilerMessage Warning
| WMDisable ->
Expand Down
2 changes: 1 addition & 1 deletion src/context/common.ml
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ let create compilation_step cs version args =
get_macros = (fun() -> None);
info = (fun ?depth _ _ -> die "" __LOC__);
warning = (fun ?depth _ _ _ -> die "" __LOC__);
warning_options = [];
warning_options = [List.map (fun w -> {wo_warning = w;wo_mode = WMDisable}) WarningList.disabled_warnings];
error = (fun ?depth _ _ -> die "" __LOC__);
located_error = (fun ?depth _ -> die "" __LOC__);
get_messages = (fun() -> []);
Expand Down
2 changes: 1 addition & 1 deletion src/context/typecore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ let pass_name = function
| PFinal -> "final"

let warning ?(depth=0) ctx w msg p =
let options = (Warning.from_meta ctx.curclass.cl_meta) @ (Warning.from_meta ctx.curfield.cf_meta) in
let options = (Warning.from_meta ctx.curfield.cf_meta) @ (Warning.from_meta ctx.curclass.cl_meta) in
ctx.com.warning ~depth w options msg p

let make_call ctx e el t p = (!make_call_ref) ctx e el t p
Expand Down
19 changes: 11 additions & 8 deletions src/core/warning.ml
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,20 @@ let get_mode w (l : warning_option list list) =
| None -> false
| Some w' -> matches w' id
in
let rec loop mode l = match l with
let rec loop l = match l with
| [] ->
mode
WMEnable
| l2 :: l ->
let rec loop2 mode l = match l with
let rec loop2 l = match l with
| [] ->
mode
None
| opt :: l ->
let mode = if matches w opt.wo_warning then opt.wo_mode else mode in
loop2 mode l
if matches w opt.wo_warning then Some opt.wo_mode else loop2 l
in
loop (loop2 mode l2) l
match loop2 l2 with
| None ->
loop l
| Some mode ->
mode
in
loop WMEnable (* ? *) l
loop l
11 changes: 8 additions & 3 deletions src/typing/operators.ml
Original file line number Diff line number Diff line change
Expand Up @@ -347,12 +347,17 @@ let make_binop ctx op e1 e2 is_assign_op with_type p =
with Error (Unify _,_,_) ->
e1,AbstractCast.cast_or_unify ctx e1.etype e2 p
in
if not ctx.com.config.pf_supports_function_equality then begin match e1.eexpr, e2.eexpr with
begin match e1.eexpr, e2.eexpr with
| TConst TNull , _ | _ , TConst TNull -> ()
| _ ->
match follow e1.etype, follow e2.etype with
| TFun _ , _ | _, TFun _ -> warning ctx WClosureCompare "Comparison of function values is unspecified on this target, use Reflect.compareMethods instead" p
| _ -> ()
| TFun _ , _ | _, TFun _ when not ctx.com.config.pf_supports_function_equality ->
warning ctx WClosureCompare "Comparison of function values is unspecified on this target, use Reflect.compareMethods instead" p
| TEnum(en,_), _ | _, TEnum(en,_) ->
if not (Meta.has Meta.FlatEnum en.e_meta) then
warning ctx WUnsafeEnumEquality "Equality operations on this enum might lead to unexpected results because some constructors have arguments" p
| _ ->
()
end;
mk_op e1 e2 ctx.t.tbool
| OpGt
Expand Down
21 changes: 21 additions & 0 deletions tests/misc/projects/Issue11813/Main.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
enum FlatEnum {
A;
B;
}

enum ThickEnum {
C;
DD(i:Int);
}

function main() {
var flat1 = A;
var flat2 = B;
if (flat1 == flat2) {}
if (flat1 != flat2) {}

var thick1 = C;
var thick2 = DD(1);
if (thick1 == thick2) {}
if (thick1 != thick2) {}
}
2 changes: 2 additions & 0 deletions tests/misc/projects/Issue11813/compile-without.hxml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-m Main
--interp
Empty file.
3 changes: 3 additions & 0 deletions tests/misc/projects/Issue11813/compile.hxml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-m Main
-w +WUnsafeEnumEquality
--interp
2 changes: 2 additions & 0 deletions tests/misc/projects/Issue11813/compile.hxml.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Main.hx:19: characters 6-22 : Warning : (WUnsafeEnumEquality) Equality operations on this enum might lead to unexpected results because some constructors have arguments
Main.hx:20: characters 6-22 : Warning : (WUnsafeEnumEquality) Equality operations on this enum might lead to unexpected results because some constructors have arguments

0 comments on commit 08243f6

Please sign in to comment.