Permalink
Browse files

src: activating as many warnings as possible

to make sure that for the files that respect them won't be broken
by future commits
  • Loading branch information...
1 parent 805952a commit 283734f2fae72f2d2c04c448c74d6e7d82df5d16 @sliquister sliquister committed with gasche Mar 11, 2012
Showing with 20 additions and 9 deletions.
  1. +10 −0 myocamlbuild.ml
  2. +2 −1 src/_tags
  3. +2 −2 src/batMap.ml
  4. +1 −1 src/batReturn.ml
  5. +1 −1 src/batSet.ml
  6. +3 −3 src/batString.ml
  7. +1 −1 src/batUnit.ml
View
@@ -92,6 +92,16 @@ let _ = dispatch begin function
end
| After_rules ->
+
+ for n = 1 to 30 do
+ List.iter (fun symbol ->
+ flag ["ocaml"; "compile"; Printf.sprintf "warn_%s%d" symbol n]
+ (S[A"-w"; A (Printf.sprintf "%s%d" symbol n)]);
+ flag ["ocaml"; "compile"; Printf.sprintf "warn_error_%s%d" symbol n]
+ (S[A"-warn-error"; A (Printf.sprintf "%s%d" symbol n)])
+ ) ["+"; "-"; "@"]
+ done;
+
(* When one links an OCaml program, one should use -linkpkg *)
flag ["ocaml"; "link"; "program"] & A"-linkpkg";
View
@@ -3,4 +3,5 @@
"syntax/pa_comprehension" or "syntax/pa_strings": include
<batFingerTree.ml>: rectypes, warn_A, warn_e
true: debug
-<{batMap,batVect,batFile,batPervasives,batParserCo,batSet}.ml>: warn_z
+true: warn_A, warn_-6, warn_e, warn_-9
+<{batMap,batVect,batFile,batPervasives,batParserCo,batSet,batLogger,batPathGen,batSplay}.ml>: warn_z
View
@@ -901,7 +901,7 @@ let foldi = Concrete.foldi
(*$Q foldi
(Q.list Q.small_int) (fun xs -> \
let m = List.fold_left (fun acc x -> add x true acc) empty xs in \
- foldi (fun x y acc -> x :: acc) m [] |> List.rev = List.sort_unique Int.compare xs)
+ foldi (fun x _y acc -> x :: acc) m [] |> List.rev = List.sort_unique Int.compare xs)
*)
let enum = Concrete.enum
@@ -1047,7 +1047,7 @@ module PMap = struct (*$< PMap *)
(*$Q foldi
(Q.list Q.small_int) (fun xs -> \
let m = List.fold_left (fun acc x -> add x true acc) (create Int.compare) xs in \
- foldi (fun x y acc -> x :: acc) m [] |> List.rev = List.sort_unique Int.compare xs)
+ foldi (fun x _y acc -> x :: acc) m [] |> List.rev = List.sort_unique Int.compare xs)
*)
let enum t = Concrete.enum t.map
View
@@ -33,6 +33,6 @@ let with_label = label
(* testing nesting with_labels *)
(*$T with_label
with_label (fun label1 -> \
- with_label (fun label2 -> ignore (return label1 1)); 2 \
+ with_label (fun _label2 -> ignore (return label1 1)); 2 \
) = 1
*)
View
@@ -600,7 +600,7 @@ let fold f s acc = Concrete.fold f s acc
let map f s = Concrete.map Pervasives.compare f s
(*$T map
- map (fun x -> 1) (of_list [1;2;3]) |> cardinal = 1
+ map (fun _x -> 1) (of_list [1;2;3]) |> cardinal = 1
*)
let filter f s = Concrete.filter Pervasives.compare f s
View
@@ -119,7 +119,7 @@ let find_from str pos sub =
find_from "foobarbaz" 7 "" = 7
try ignore (find_from "" 0 "a"); false with Not_found -> true
try ignore (find_from "foo" 2 "foo"); false with Not_found -> true
- try ignore (find_from "foo" 3 "foo"); false with Not_found _ -> true
+ try ignore (find_from "foo" 3 "foo"); false with Not_found -> true
try ignore (find_from "foo" 4 "foo"); false with Invalid_argument _ -> true
try ignore (find_from "foo" (-1) "foo"); false with Invalid_argument _ -> true
*)
@@ -459,7 +459,7 @@ let of_enum e =
s
(*$T of_enum
Enum.init 3 (fun i -> char_of_int (i + int_of_char '0')) |> of_enum = "012"
- Enum.init 0 (fun i -> ' ') |> of_enum = ""
+ Enum.init 0 (fun _i -> ' ') |> of_enum = ""
*)
let of_backwards e =
@@ -552,7 +552,7 @@ let iteri f str =
iteri count_letter word;
Array.mapi (fun c pos -> (char_of_int c, List.rev pos)) positions
|> Array.to_list
- |> List.filter (fun (c,pos) -> pos <> [])
+ |> List.filter (fun (_c, pos) -> pos <> [])
in
assert_equal ~msg:"String.iteri test"
(letter_positions "hello")
View
@@ -28,6 +28,6 @@ let of_string = function
| "()" -> ()
| _ -> raise (Invalid_argument "unit_of_string")
let compare () () = 0
-let print out t = BatInnerIO.nwrite out unit_string
+let print out () = BatInnerIO.nwrite out unit_string
(*BISECT-IGNORE-END*)

5 comments on commit 283734f

@vincent-hugot
Member

This commit appears to have completely broken compilation, at least on my system.

From a fresh clone, I get the following error from 232792e on, but not in its father 805952a (the duplicate history makes it more confusing than it needs to be).

Σ batteries-included ⌛7 ✘130 make all test                                                                                                                                 ~/Desktop/Σ 14:00 vhugot@seth master: ⌘
Build mode: shared
ocamlbuild -no-links syntax.otarget src/batteries.cma src/batteriesHelp.cmo src/batteriesThread.cma META src/batteries.cmxs src/batteriesThread.cmxs
Finished, 1 target (0 cached) in 00:00:00.
ocamlfind: Package `bisect' not found
+ ocamlfind ocamlc -c -g -annot -w A -w e -w -29 -warn-error A -package bigarray,num,str -package camlp4.lib -pp camlp4of -pp camlp4of -I src/syntax/pa_strings -I build -I src -I qtest -I benchsuite -I libs -I testsuite -I src/syntax/pa_comprehension -I libs/estring -o src/syntax/pa_strings/pa_strings.cmo src/syntax/pa_strings/pa_strings.ml
File "ghost-location", line 74, characters 9--2103:
Warning 27: unused variable ctx.
File "ghost-location", line 86, characters 9--2530:
Warning 27: unused variable ctx.
File "ghost-location", line 103, characters 9--3289:
Warning 27: unused variable ctx.
File "src/syntax/pa_strings/pa_strings.ml", line 1, characters 0-1:
Error: Error-enabled warnings (3 occurrences)
Command exited with code 2.
Compilation unsuccessful after building 10 targets (9 cached) in 00:00:00.
make: *** [all] Error 10
@gasche
Member
gasche commented on 283734f Mar 13, 2012

Indeed. I have a patch for some of those warnings (the ones for syntax extensions) but some of the later ones are more delicate and would require care (there is no point in suppressing warnings to suppress warnings, one should audit the code first). I forgot to push it this morning. I will do that just now. I suggest we revert this patch while we don't understand what's happening.

In any case, I think activating "-warn-error A" is a bad idea, because it makes fail when using a new version of OCaml that introduces new warning -- that is a very realistic scenario as it happened for 3.12.

@sliquister
Contributor

And I didn't activate the warnings in some files, because I wouldn't have been able to tell for all
if they were justified or not.
I did run make clean; make test before pushing so I don't know why I didn't see that.
Perhaps these commands do not compile everything.

@sliquister
Contributor

And the problem is more warn A than warn-error A.

@gasche
Member
gasche commented on 283734f Mar 13, 2012

And the problem is more warn A than warn-error A.

I don't understand. In my book seeing warnings printed is fine, but breaking compilation is not. We may want to either remove the source of the warning or disable the warning (as selectively as possible), but the warning don't hurt. Abuse of -warn-error, on the contrary, does, because "not building" is the most serious bug of all.

I wouldn't object to have continuous testing instances doing build with -warn-error A, just to make sure we don't miss new warnings (they tend to be forgotten once the compiled files are in cache). But it's impractical already for developpers -- whereas they should assume their deeds and see the warnings -- and just doesn't work for early adopters / testers / potential contributors.

Please sign in to comment.