[tests] kill some new dead code

Summary:
Make dead code detection part of `make test` so that dead code stops creeping
in. It's only enabled if all the analysers are enabled and if this is a
facebook build, because the dead code detection will have false positives
otherwise.

Reviewed By: mbouaziz

Differential Revision: D6807395

fbshipit-source-id: ebbd835

.gitignore

@@ -163,3 +163,5 @@ infer/src/.project
 # generated when looking for dead code
 /infer/src/deadcode/.depend
 /infer/src/deadcode/jbuild
+/infer/src/deadcode/*.ml
+/infer/src/deadcode/*.mli

Makefile

@@ -181,6 +181,12 @@ test_build: src_build_common
 	$(QUIET)$(call silent_on_success,Testing Infer builds without warnings,\
 	$(MAKE_SOURCE) test)
 
+# depend on test_build so that we do not run them in parallel
+.PHONY: deadcode
+deadcode: src_build_common test_build
+	$(QUIET)$(call silent_on_success,Testing there is no dead OCaml code,\
+	$(MAKE) -C $(SRC_DIR)/deadcode)
+
 .PHONY: toplevel
 toplevel: src_build_common
 	$(QUIET)$(call silent_on_success,Building Infer REPL,\
@@ -392,6 +398,11 @@ test: crash_if_not_all_analyzers_enabled config_tests
 ifeq (,$(findstring s,$(MAKEFLAGS)))
 	$(QUIET)echo "$(TERM_INFO)ALL TESTS PASSED$(TERM_RESET)"
 endif
+ifeq ($(IS_FACEBOOK_TREE),yes)
+ifneq ($(GNU_SED),no)
+test: deadcode
+endif
+endif
 
 .PHONY: quick-test
 quick-test: test_build ocaml_unit_test

Makefile.autoconf.in

@@ -29,6 +29,7 @@ EMACS = @EMACS@
 ENABLE_OCAMLOPT_CUSTOM_CC = @ENABLE_OCAMLOPT_CUSTOM_CC@
 ENABLE_OCAML_BINANNOT = @ENABLE_OCAML_BINANNOT@
 exec_prefix = @exec_prefix@
+GNU_SED = @GNU_SED@
 INFER_MAJOR = @INFER_MAJOR@
 INFER_MAN_LAST_MODIFIED = @INFER_MAN_LAST_MODIFIED@
 INFER_MINOR = @INFER_MINOR@

configure.ac

@@ -316,6 +316,19 @@ AC_CHECK_PYTHON_MODULE([$PYTHON27], [lxml])
 
 AC_CHECK_TOOL([XCPRETTY], [xcpretty], [no])
 
+AC_CHECK_TOOL([SED], [sed], [no])
+AS_IF([test "$SED" != "xno"], [
+  AC_MSG_CHECKING([if sed is GNU sed])
+  AS_IF(["$SED" --version 2> /dev/null | grep -q -e "GNU sed"], [
+    GNU_SED="$SED"
+    AC_MSG_RESULT([yes])
+  ], [
+    AC_MSG_RESULT([no])
+    AC_CHECK_TOOL([GNU_SED], [gsed], [no])
+  ])
+])
+AC_SUBST([GNU_SED])
+
 AC_CHECK_TOOL([BREW], [brew], [no])
 
 # warn about broken pkg-config version for brew users

infer/src/IR/Cfg.mli

@@ -21,9 +21,6 @@ val load : SourceFile.t -> t option
 val store : SourceFile.t -> t -> unit
 (** Save a cfg into the database *)
 
-val get_all_proc_descs : t -> Procdesc.t list
-(** get all the values from the hashtable *)
-
 val get_all_proc_names : t -> Typ.Procname.t list
 (** get all the keys from the hashtable *)
 

infer/src/Makefile

@@ -95,7 +95,7 @@ all: infer
 
 GENERATED_FROM_AUTOCONF = jbuild.common jbuild-workspace base/Version.ml
 
-GENERATED_JBUILDS = jbuild atd/jbuild deadcode/jbuild istd/jbuild scripts/jbuild
+GENERATED_JBUILDS += jbuild atd/jbuild istd/jbuild scripts/jbuild
 
 SRC_BUILD_COMMON = $(GENERATED_FROM_AUTOCONF) $(GENERATED_JBUILDS) $(OCAML_SOURCES)
 ifeq ($(BUILD_C_ANALYZERS),yes)

infer/src/base/DB.ml

@@ -78,20 +78,14 @@ let source_dir_from_source_file source_file =
 
 type filename = string [@@deriving compare]
 
-let equal_filename = [%compare.equal : filename]
-
 let filename_to_string s = s
 
 let filename_from_string s = s
 
 let filename_add_suffix fn s = fn ^ s
 
-let chop_extension = Filename.chop_extension
-
 let file_exists path = Sys.file_exists path = `Yes
 
-let file_remove = Sys.remove
-
 module FilenameSet = Caml.Set.Make (struct
   type t = filename [@@deriving compare]
 end)

infer/src/base/DB.mli

@@ -17,8 +17,6 @@ open! IStd
 (** generic file name *)
 type filename [@@deriving compare]
 
-val equal_filename : filename -> filename -> bool
-
 module FilenameSet : Caml.Set.S with type elt = filename
 
 module FilenameMap : Caml.Map.S with type key = filename
@@ -27,14 +25,10 @@ val filename_from_string : string -> filename
 
 val filename_to_string : filename -> string
 
-val chop_extension : filename -> filename
-
 val filename_add_suffix : filename -> string -> filename
 
 val file_exists : filename -> bool
 
-val file_remove : filename -> unit
-
 val file_modified_time : ?symlink:bool -> filename -> float
 (** Return the time when a file was last modified. The file must exist. *)
 

infer/src/base/Serialization.ml

@@ -22,9 +22,9 @@ module Key = struct
   (** Serialization key, used to distinguish versions of serializers and avoid assert faults *)
   type t = int
 
-  (** current key for tenv, procedure summary, cfg, error trace, call graph *)
-  let tenv, summary, cg, analysis_results, cluster, lint_issues =
-    (425184201, 160179325, 477305409, 799050016, 579094948, 852343110)
+  (** Current keys for various serializable objects. The keys are computed using the [generate_keys] function below *)
+  let tenv, summary, analysis_results, cluster, lint_issues =
+    (425184201, 160179325, 799050016, 579094948, 852343110)
 end
 
 (** version of the binary files, to be incremented for each change *)
@@ -122,11 +122,10 @@ let read_from_file s = s.read_from_file
 
 let write_to_file s = s.write_to_file
 
-(*
-(** Generate random keys, to be used in an ocaml toplevel *)
+(** Generate new (random) serialization keys, to be run in an ocaml toplevel and used in the [Key]
+    module above *)
 let generate_keys () =
-  Random.self_init ();
+  Random.self_init () ;
   let max_rand_int = 0x3FFFFFFF (* determined by Rand library *) in
   let gen () = Random.int max_rand_int in
-  gen (), gen (), gen (), gen (), gen (), gen ()
-*)
+  (gen (), gen (), gen (), gen (), gen ())

infer/src/base/Serialization.mli

@@ -19,9 +19,6 @@ module Key : sig
   val analysis_results : t
   (** current key for an analysis results value *)
 
-  val cg : t
-  (** current key for a call graph *)
-
   val cluster : t
   (** current key for a cluster *)
 
@@ -50,3 +47,7 @@ val read_from_string : 'a serializer -> string -> 'a option
 
 val write_to_file : 'a serializer -> data:'a -> DB.filename -> unit
 (** Serialize into a file writing value *)
+
+val generate_keys : unit -> int * int * int * int * int
+  [@@warning "-32"]
+(** Generate new (random) serialization keys, to be used in an ocaml toplevel *)

infer/src/checkers/annotations.ml

@@ -168,8 +168,6 @@ let ia_is_nullable ia = ia_ends_with ia nullable || ia_is_propagates_nullable ia
 
 let ia_is_present ia = ia_ends_with ia present
 
-let ia_is_prop ia = ia_ends_with ia prop
-
 let ia_is_nonnull ia = List.exists ~f:(ia_ends_with ia) [nonnull; notnull; camel_nonnull]
 
 let ia_is_false_on_null ia = ia_ends_with ia false_on_null

infer/src/checkers/annotations.mli

@@ -96,8 +96,6 @@ val ia_is_on_bind : Annot.Item.t -> bool
 
 val ia_is_on_mount : Annot.Item.t -> bool
 
-val ia_is_prop : Annot.Item.t -> bool
-
 val ia_is_on_unbind : Annot.Item.t -> bool
 
 val ia_is_on_unmount : Annot.Item.t -> bool

infer/src/deadcode/Makefile

@@ -30,11 +30,15 @@
 ROOT_DIR = ../../..
 include $(ROOT_DIR)/Makefile.config
 
+INFER_BUILD_DIR = ../_build/test
+
+ALL_INFER_IN_ONE_FILE_ML = all_infer_in_one_file.ml
+
 default: detect_dead_code
 
-ml_src_files_from_mlly:=$(shell find .. -not -path "../*stubs*" -regex '\.\./[a-zA-Z].*\.ml\(l\|y\)')
+ml_src_files_from_mlly:=$(shell find .. -not -path "../*stubs*" -regex '\.\./[a-zA-Z].*\.ml[ly]')
 
-ml_src_files:=$(shell cd .. && find . -not -path "./*stubs*" -regex '\./[a-zA-Z].*\.ml\(i\)*')
+ml_src_files:=$(shell cd .. && find . -not -path "./*stubs*" -regex '\./[a-zA-Z].*\.mli*')
 
 ml_src_files_without_mli:=$(shell cd .. && for i in */*.ml */*/*.ml; do [ -f $${i}i ] || echo $$i; done)
 
@@ -52,79 +56,91 @@ depend:
 #	file, you need the module value to be defined before you can refer to it, even in
 #	signatures. Because of this, the order given by ocamldep is not enough to avoid "Unbound
 #	module MyModule" errors in the case of signatures referring to other modules.
-	$(QUIET)echo "(* START OF SIGNATURE $*.mli *)" >> all_infer_in_one_file.ml
+	$(QUIET)echo "(* START OF SIGNATURE $*.mli *)" >> $(ALL_INFER_IN_ONE_FILE_ML)
 #	put too many spaces in general but you never know what the contents of the file is;
 #	sometimes spaces will be needed
-	$(QUIET)echo "  module type " >> all_infer_in_one_file.ml
+	$(QUIET)echo "  module type " >> $(ALL_INFER_IN_ONE_FILE_ML)
 #	suppress some warnings for generated code
-	$(QUIET)if [[ $@ =~ (atd|deadcode)/ ]]; then echo '  [@warning "-27-32-34-35-39"] ' >> all_infer_in_one_file.ml; fi
+	$(QUIET)if [[ $@ =~ (atd|deadcode)/ ]]; then echo '  [@warning "-27-32-34-35-39"] ' >> $(ALL_INFER_IN_ONE_FILE_ML); fi
 #	compute module name from file name: capitalize first letter
-	$(QUIET)echo $(shell basename $*) | sed -e "s/\b\(.\)/  \u\1/g" >> all_infer_in_one_file.ml
-	$(QUIET)echo "  = sig " >> all_infer_in_one_file.ml
+	$(QUIET)echo $(shell basename $*) | $(GNU_SED) -e "s/\b\(.\)/  \u\1/g" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  = sig " >> $(ALL_INFER_IN_ONE_FILE_ML)
 #	pre-processor directive to get errors in the original files and not in all_infer_in_one_file.ml
-	$(QUIET)echo '# 1 "$*.mli"' >> all_infer_in_one_file.ml
-	cat ../$*.mli >> all_infer_in_one_file.ml
-	$(QUIET)echo "  end " >> all_infer_in_one_file.ml
-	$(QUIET)echo "(* END OF SIGNATURE $*.mli *)" >> all_infer_in_one_file.ml
-	$(QUIET)echo >> all_infer_in_one_file.ml
+	$(QUIET)echo '# 1 "$*.mli"' >> $(ALL_INFER_IN_ONE_FILE_ML)
+	cat ../$*.mli >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  end " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "(* END OF SIGNATURE $*.mli *)" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo >> $(ALL_INFER_IN_ONE_FILE_ML)
 #	lots of duplication from above, sorry
-	$(QUIET)echo "(* START OF MODULE $*.ml *)" >> all_infer_in_one_file.ml
-	$(QUIET)echo "  module " >> all_infer_in_one_file.ml
-	$(QUIET)if [[ $@ =~ (atd|deadcode)/ ]]; then echo '  [@warning "-27-32-34-35-39"] ' >> all_infer_in_one_file.ml; fi
-	$(QUIET)echo $(shell basename $*) | sed -e "s/\b\(.\)/  \u\1/g" >> all_infer_in_one_file.ml
-	$(QUIET)echo "  : " >> all_infer_in_one_file.ml
-	$(QUIET)echo $(shell basename $*) | sed -e "s/\b\(.\)/  \u\1/g" >> all_infer_in_one_file.ml
-	$(QUIET)echo "  = struct " >> all_infer_in_one_file.ml
-	$(QUIET)echo '# 1 "$*.ml"' >> all_infer_in_one_file.ml
-	cat ../$*.ml >> all_infer_in_one_file.ml
-	$(QUIET)echo "  end " >> all_infer_in_one_file.ml
-	$(QUIET)echo "(* END OF MODULE $*.ml *)" >> all_infer_in_one_file.ml
-	$(QUIET)echo >> all_infer_in_one_file.ml
+	$(QUIET)echo "(* START OF MODULE $*.ml *)" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  module " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)if [[ $@ =~ (atd|deadcode)/ ]]; then echo '  [@warning "-27-32-34-35-39"] ' >> $(ALL_INFER_IN_ONE_FILE_ML); fi
+	$(QUIET)echo $(shell basename $*) | $(GNU_SED) -e "s/\b\(.\)/  \u\1/g" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  : " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo $(shell basename $*) | $(GNU_SED) -e "s/\b\(.\)/  \u\1/g" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  = struct " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo '# 1 "$*.ml"' >> $(ALL_INFER_IN_ONE_FILE_ML)
+	cat ../$*.ml >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  end " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "(* END OF MODULE $*.ml *)" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo >> $(ALL_INFER_IN_ONE_FILE_ML)
 
 $(ml_src_files_without_mli:.ml=.cmx):
 #	again mostly duplicated from above
-	$(QUIET)echo "(* START OF MODULE $(@) *)" >> all_infer_in_one_file.ml
-	$(QUIET)echo "  module " >> all_infer_in_one_file.ml
-	$(QUIET)if [[ $@ =~ (atd|deadcode)/ ]]; then echo '  [@warning "-27-32-34-35-39"] ' >> all_infer_in_one_file.ml; fi
-	$(QUIET)echo $(shell basename $@ .cmx) | sed -e "s/\b\(.\)/  \u\1/g" >> all_infer_in_one_file.ml
-	$(QUIET)echo "  = struct " >> all_infer_in_one_file.ml
-	$(QUIET)echo "# 1 \"$$(echo $@ | sed -e 's/\.cmx$$/.ml/')\"" >> all_infer_in_one_file.ml
-	cat ../$$(echo $@ | sed -e "s/\.cmx$$/.ml/") >> all_infer_in_one_file.ml
-	$(QUIET)echo "  end " >> all_infer_in_one_file.ml
-	$(QUIET)echo "(* END OF MODULE $@ *)" >> all_infer_in_one_file.ml
-	$(QUIET)echo >> all_infer_in_one_file.ml
+	$(QUIET)echo "(* START OF MODULE $(@) *)" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  module " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)if [[ $@ =~ (atd|deadcode)/ ]]; then echo '  [@warning "-27-32-34-35-39"] ' >> $(ALL_INFER_IN_ONE_FILE_ML); fi
+	$(QUIET)echo $(shell basename $@ .cmx) | $(GNU_SED) -e "s/\b\(.\)/  \u\1/g" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  = struct " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "# 1 \"$$(echo $@ | $(GNU_SED) -e 's/\.cmx$$/.ml/')\"" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	cat ../$$(echo $@ | $(GNU_SED) -e "s/\.cmx$$/.ml/") >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "  end " >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo "(* END OF MODULE $@ *)" >> $(ALL_INFER_IN_ONE_FILE_ML)
+	$(QUIET)echo >> $(ALL_INFER_IN_ONE_FILE_ML)
 
 -include .depend
 
-.PHONY: all_infer_in_one_file.ml
-all_infer_in_one_file.ml: backend/infer.cmx unit/inferunit.cmx facebook/InferCreateTraceViewLinks.cmx
-	$(QUIET)echo "see results in all_infer_in_one_file.ml"
+# Concatenate all source files of infer into a single file. Assume that all source files are
+# available (in particular generated ones) and .depend has been created by ocamldep. Depend on the
+# root .cmx to include all the code. Any code not used in the construction of these "root .cmx" will
+# be considered dead.
+.PHONY: flatten_infer
+flatten_infer: backend/infer.cmx unit/inferunit.cmx facebook/InferCreateTraceViewLinks.cmx
+	$(QUIET)echo "see results in $(ALL_INFER_IN_ONE_FILE_ML)"
 
 .PHONY: detect_dead_code
 detect_dead_code:
 	$(MAKE) clean
+#	create a dummy implementation file to keep jbuilder happy, as we are about to generate the
+#	jbuilder file for this directory
+	touch $(ALL_INFER_IN_ONE_FILE_ML) $(ALL_INFER_IN_ONE_FILE_ML:.ml=.mli)
 #	needed to get jbuild generated, and the generated code for the lexers and parsers in ../_build
-	$(MAKE) -C .. byte
+	$(MAKE) GENERATED_JBUILDS=deadcode/jbuild -C .. test
 #	copy generated source files from ../_build
 	for file in $(ml_src_files_from_mlly); do \
 	  set +e; \
-	  [ -f "../_build/default/$$(basename $$file .mly).ml" ] && \
-	    $(COPY) ../_build/default/$$(basename $$file .mly).ml .; \
-	  [ -f "../_build/default/$$(basename $$file .mly).mli" ] && \
-	    $(COPY) ../_build/default/$$(basename $$file .mly).mli .; \
-	  [ -f "../_build/default/$$(basename $$file .mll).ml" ] && \
-	    $(COPY) ../_build/default/$$(basename $$file .mll).ml .; \
-	  [ -f "../_build/default/$$(basename $$file .mll).mli" ] && \
-	    $(COPY) ../_build/default/$$(basename $$file .mll).mli .; \
+	  [ -f "$(INFER_BUILD_DIR)/$$(basename $$file .mly).ml" ] && \
+	    $(COPY) $(INFER_BUILD_DIR)/$$(basename $$file .mly).ml .; \
+	  [ -f "$(INFER_BUILD_DIR)/$$(basename $$file .mly).mli" ] && \
+	    $(COPY) $(INFER_BUILD_DIR)/$$(basename $$file .mly).mli .; \
+	  [ -f "$(INFER_BUILD_DIR)/$$(basename $$file .mll).ml" ] && \
+	    $(COPY) $(INFER_BUILD_DIR)/$$(basename $$file .mll).ml .; \
+	  [ -f "$(INFER_BUILD_DIR)/$$(basename $$file .mll).mli" ] && \
+	    $(COPY) $(INFER_BUILD_DIR)/$$(basename $$file .mll).mli .; \
 	  set -e; \
 	done
-	$(REMOVE) all_infer_in_one_file.ml
 	$(MAKE) depend
 #	Need to be sequential to avoid getting a garbled file. Need to re-include .depend as it may
-#	have changed, so run another `make`.
-	$(MAKE) -j 1 all_infer_in_one_file.ml
-#	build and get dead code warnings
-	jbuilder build ../_build/default/deadcode/all_infer_in_one_file.bc
+#	have changed. For both of these reasons, run another `make`.
+#	Create a temp file so that the build doesn't break if this step gets interrupted.
+	tmp_file=$$(mktemp --tmpdir all_infer_in_one_file_XXXXX.ml); \
+	$(MAKE) -j 1 ALL_INFER_IN_ONE_FILE_ML="$$tmp_file" flatten_infer; \
+	mv "$$tmp_file" $(ALL_INFER_IN_ONE_FILE_ML)
+#	build and get dead code warnings; clean in case of errors so as not to leave rubbish around
+	if ! jbuilder build $(INFER_BUILD_DIR)/deadcode/all_infer_in_one_file.bc; then \
+	  $(MAKE) clean; \
+	  exit 1; \
+	fi
 #	be paranoid about cleaning because we do not want to include infer_in_one_file into infer by
 #	accident and I don't know enough jbuilder to be positive that it won't happen
 	$(MAKE) clean
@@ -132,4 +148,3 @@ detect_dead_code:
 .PHONY: clean
 clean:
 	$(REMOVE) .depend *.ml *.mli jbuild
-	touch all_infer_in_one_file.mli all_infer_in_one_file.ml

scripts/dead_code_detection.sh

@@ -1,19 +0,0 @@
-#!/bin/bash
-
-# Copyright (c) 2018 - present Facebook, Inc.
-# All rights reserved.
-#
-# This source code is licensed under the BSD style license found in the
-# LICENSE file in the root directory of this source tree. An additional grant
-# of patent rights can be found in the PATENTS file in the same directory.
-
-
-# Dead code detection for infer's source code. See comments in infer/src/deadcode/Makefile.
-
-set -e
-set -o pipefail
-set -u
-
-SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
-
-make -C "$SCRIPT_DIR"/../infer/src/deadcode