Skip to content

Commit

Permalink
Re-land "D6384244: Remove ServerEnv.failed_[parsing|decl|check] sets..."
Browse files Browse the repository at this point in the history
Summary: It was reverted due to missing update to stubs/ai.ml that is used in Open Source build.

Reviewed By: alexchow

Differential Revision: D6730499

fbshipit-source-id: 5cf237663abfbc9d8f91db10054b4c212cf291af
  • Loading branch information
dabek authored and hhvm-bot committed Jan 17, 2018
1 parent c635aa5 commit 8b1b511
Show file tree
Hide file tree
Showing 28 changed files with 134 additions and 237 deletions.
44 changes: 12 additions & 32 deletions hphp/hack/src/decl/decl_redecl_service.ml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ open Typing_deps
(*****************************************************************************)
(* The neutral element of declaration (cf procs/multiWorker.mli) *)
(*****************************************************************************)
let otf_neutral = Errors.empty, Relative_path.Set.empty
let otf_neutral = Errors.empty
let compute_deps_neutral = DepSet.empty, DepSet.empty

(*****************************************************************************)
Expand All @@ -44,30 +44,11 @@ end)
(* Re-declaring the types in a file *)
(*****************************************************************************)

(* Returns a list of files that are considered to have failed decl and must
* be redeclared every time the typechecker discovers a file change *)
let get_decl_failures decl_errors fn =
List.fold_left decl_errors ~f:begin fun failed error ->
(* It is important to add the file that is the cause of the failure.
* What can happen is that during a declaration phase, we realize
* that a parent class is outdated. When this happens, we redeclare
* the class, even if it is in a different file. Therefore, the file
* where the error occurs might be different from the file we
* are declaring right now.
*)
let file_with_error = Pos.filename (Errors.get_pos error) in
assert (file_with_error <> Relative_path.default);
let failed = Relative_path.Set.add failed file_with_error in
let failed = Relative_path.Set.add failed fn in
failed
end ~init:Relative_path.Set.empty

let on_the_fly_decl_file tcopt (errors, failed) fn =
let decl_errors, (), _ = Errors.do_with_context fn Errors.Decl begin fun () ->
let on_the_fly_decl_file tcopt errors fn =
let decl_errors, () = Errors.do_with_context fn Errors.Decl begin fun () ->
Decl.make_env tcopt fn
end in
let failed' = get_decl_failures (Errors.get_error_list decl_errors) fn in
Errors.merge decl_errors errors, Relative_path.Set.union failed failed'
Errors.merge decl_errors errors

(*****************************************************************************)
(* Given a set of classes, compare the old and the new type and deduce
Expand Down Expand Up @@ -130,13 +111,12 @@ let compute_gconsts_deps old_gconsts (to_redecl, to_recheck) gconsts =
let redeclare_files tcopt filel =
List.fold_left filel
~f:(on_the_fly_decl_file tcopt)
~init:(Errors.empty, Relative_path.Set.empty)
~init:Errors.empty

let otf_decl_files tcopt filel =
SharedMem.invalidate_caches();
(* Redeclaring the files *)
let errors, failed = redeclare_files tcopt filel in
errors, failed
redeclare_files tcopt filel

let compute_deps fast filel =
let infol =
Expand Down Expand Up @@ -188,8 +168,8 @@ let load_and_compute_deps _acc filel =
(* Merges the results coming back from the different workers *)
(*****************************************************************************)

let merge_on_the_fly (errorl1, failed1) (errorl2, failed2) =
Errors.merge errorl1 errorl2, Relative_path.Set.union failed1 failed2
let merge_on_the_fly errorl1 errorl2 =
Errors.merge errorl1 errorl2

let merge_compute_deps (to_redecl1, to_recheck1) (to_redecl2, to_recheck2) =
DepSet.union to_redecl1 to_redecl2, DepSet.union to_recheck1 to_recheck2
Expand All @@ -200,7 +180,7 @@ let merge_compute_deps (to_redecl1, to_recheck1) (to_redecl2, to_recheck2) =
let parallel_otf_decl workers bucket_size tcopt fast fnl =
try
OnTheFlyStore.store (tcopt, fast);
let errors, failed =
let errors =
MultiWorker.call
workers
~job:load_and_otf_decl_files
Expand All @@ -217,7 +197,7 @@ let parallel_otf_decl workers bucket_size tcopt fast fnl =
~next:(MultiWorker.next ~max_size:bucket_size workers fnl)
in
OnTheFlyStore.clear();
errors, failed, to_redecl, to_recheck
errors, to_redecl, to_recheck
with e ->
if SharedMem.is_heap_overflow () then
Exit_status.exit Exit_status.Redecl_heap_overflow
Expand Down Expand Up @@ -374,9 +354,9 @@ let redo_type_decl workers ~bucket_size tcopt all_oldified_defs fast defs =
let result =
if List.length fnl < 10
then
let errors, failed = otf_decl_files tcopt fnl in
let errors = otf_decl_files tcopt fnl in
let to_redecl, to_recheck = compute_deps fast fnl in
errors, failed, to_redecl, to_recheck
errors, to_redecl, to_recheck
else parallel_otf_decl workers bucket_size tcopt fast fnl
in
remove_old_defs defs all_elems;
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/decl/decl_redecl_service.mli
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ val redo_type_decl :
FileInfo.names ->
FileInfo.fast ->
FileInfo.names ->
Errors.t * Relative_path.Set.t * DepSet.t * DepSet.t
Errors.t * DepSet.t * DepSet.t

(**
* Exposed for tests only!
Expand Down
44 changes: 14 additions & 30 deletions hphp/hack/src/decl/decl_service.ml
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,11 @@
open Hh_core
open Utils

(* The set of files that failed *)
type failed = Relative_path.Set.t

(* The result expected from the service *)
type result = Errors.t * failed
type result = Errors.t

type error_info = {
errs : failed;
lazy_decl_errs: failed;
}
(* Used for lazy typechecking *)
type lazy_decl_result = Errors.t * error_info
type lazy_decl_result = Errors.t

(*****************************************************************************)
(* The place where we store the shared data in cache *)
Expand All @@ -45,38 +38,29 @@ end)
(* The job that will be run on the workers *)
(*****************************************************************************)

let decl_file tcopt (errorl, failed) fn =
let errorl', (), _ = Errors.do_with_context fn Errors.Decl begin fun () ->
let decl_file tcopt errorl fn =
let errorl', () = Errors.do_with_context fn Errors.Decl begin fun () ->
d ("Typing decl: "^Relative_path.to_absolute fn);
Decl.make_env tcopt fn;
dn "OK";
end
in
let failed =
if Errors.is_empty errorl' then failed
else Relative_path.Set.add failed fn in
let errorl = Errors.merge errorl' errorl in
errorl, failed
Errors.merge errorl' errorl

let decl_files (errors, failed) fnl =
let decl_files errors fnl =
let tcopt = TypeDeclarationStore.load() in
List.fold_left fnl ~f:(decl_file tcopt) ~init:(errors, failed)
List.fold_left fnl ~f:(decl_file tcopt) ~init:errors

(*****************************************************************************)
(* Merges the results (used by the master) *)
(*****************************************************************************)

let merge_decl (errors1, failed1) (errors2, failed2) =
Errors.merge errors1 errors2,
Relative_path.Set.union failed1 failed2

let merge_lazy_decl
(errors1, {errs = failed1; lazy_decl_errs = failed_decl1})
(errors2, {errs = failed2; lazy_decl_errs = failed_decl2}) =
Errors.merge errors1 errors2,
{ errs = Relative_path.Set.union failed1 failed2;
lazy_decl_errs = Relative_path.Set.union failed_decl1 failed_decl2;
}
let merge_decl errors1 errors2 =
Errors.merge errors1 errors2

let merge_lazy_decl errors1 errors2 =
Errors.merge errors1 errors2

(*****************************************************************************)
(* Let's go! That's where the action is *)
(*****************************************************************************)
Expand All @@ -85,7 +69,7 @@ let go (workers:Worker.t list option) ~bucket_size tcopt fast =
TypeDeclarationStore.store tcopt;
let fast_l =
Relative_path.Map.fold fast ~init:[] ~f:(fun x _ y -> x :: y) in
let neutral = Errors.empty, Relative_path.Set.empty in
let neutral = Errors.empty in
dn "Declaring the types";
let result =
MultiWorker.call
Expand Down
11 changes: 3 additions & 8 deletions hphp/hack/src/decl/decl_service.mli
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,11 @@
(* Module declaring the types in parallel *)
(*****************************************************************************)

(* The set of files that failed *)
type failed = Relative_path.Set.t
(* The result expected from the service *)
type result = Errors.t * failed
type error_info = {
errs : failed;
lazy_decl_errs: failed;
}
type result = Errors.t

(* Used for lazy typechecking *)
type lazy_decl_result = Errors.t * error_info
type lazy_decl_result = Errors.t

(*****************************************************************************)
(* Starts the process *)
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/format/format_hack.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ type 'a return =
let rec entry ~keep_source_metadata ~no_trailing_commas ~modes
(file : Path.t) from to_ content k =
try
let errorl, (), _ = Errors.do_ begin fun () ->
let errorl, () = Errors.do_ begin fun () ->
let rp = Relative_path.(create Dummy (file :> string)) in
let {Parser_hack.file_mode; _} =
Parser_hack.program_with_default_popt rp content in
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/full_fidelity_parse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ let handle_existing_file args filename =
let editable = Full_fidelity_editable_syntax.from_tree minimal_tree in

(* Parse with the original parser *)
let (original_errors, original_parse, _) = Errors.do_
let (original_errors, original_parse) = Errors.do_
begin
fun () -> Parser_hack.from_file ParserOptions.default file
end in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ let old_parse_with_source source path =
let _ = Pervasives.output_string channel source in
let _ = close_out channel in
let file = Relative_path.create Relative_path.Dummy path in
let errorl, result, _ =
let errorl, result =
Errors.do_ begin fun () ->
Parser_hack.from_file_with_default_popt file
end
Expand Down
4 changes: 2 additions & 2 deletions hphp/hack/src/hh_file_parse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ let print_error error = error


let parse_with_regular file content =
let errorl, parser_ret, _ =
let errorl, parser_ret =
Errors.do_ begin fun () ->
Parser_hack.program ParserOptions.default file content
end
Expand All @@ -40,7 +40,7 @@ let parse_and_print filename =
let content =
try Sys_utils.cat (Relative_path.to_absolute file) with _ -> "" in
if !verbose then Printf.printf "File contents:\n%s\n" content;
let errorl, result, _ =
let errorl, result =
Errors.do_ begin fun () ->
Fileinfo_parser.program ParserOptions.default file content
end
Expand Down
4 changes: 2 additions & 2 deletions hphp/hack/src/hh_format.ml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ let debug () fnl =
List.iter fnl begin fun (filepath : Path.t) ->
try
(* Checking that we can parse the output *)
let parsing_errors1, parser_output1, _ = Errors.do_ begin fun () ->
let parsing_errors1, parser_output1 = Errors.do_ begin fun () ->
let rp =
Relative_path.create Relative_path.Dummy (filepath :> string) in
Full_fidelity_ast.(from_file_with_legacy (make_env rp))
Expand Down Expand Up @@ -92,7 +92,7 @@ let debug () fnl =
end;

(* Checking that we can parse the output *)
let parsing_errors2, _parser_output2, _ = Errors.do_ begin fun () ->
let parsing_errors2, _parser_output2 = Errors.do_ begin fun () ->
let rp = Relative_path.(create Dummy (filepath :> string)) in
Parser_hack.program_with_default_popt rp content
end in
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/hh_single_compile.ml
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ let do_compile filename compiler_options text fail_or_ast debug_time =
SourceText.relative_pos filename source_text
(SyntaxError.start_offset e) (SyntaxError.end_offset e) in
Emit_program.emit_fatal_program ~ignore_message:false error_t pos s
| `ParseResult (errors, parser_return, _) ->
| `ParseResult (errors, parser_return) ->
let ast = parser_return.Parser_hack.ast in
List.iter (Errors.get_error_list errors) (fun e ->
P.printf "%s\n" (Errors.to_string (Errors.to_absolute e)));
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/hh_single_parse.ml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let handle_errors : Errors.t -> unit = fun errorl ->

let run_ast ?(quick=false) file =
let parse_call () = Parser_hack.from_file ParserOptions.default ~quick file in
let errorl, result, _ = Errors.do_ parse_call in
let errorl, result = Errors.do_ parse_call in
handle_errors errorl;
result

Expand Down
4 changes: 2 additions & 2 deletions hphp/hack/src/hh_single_type_check.ml
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ let test_decl_compare filename popt files_contents tcopt files_info =
~collect_garbage:false;

let files_contents = Relative_path.Map.map files_contents ~f:add_newline in
let _, _, _ = parse_name_and_decl popt files_contents tcopt in
let _, _= parse_name_and_decl popt files_contents tcopt in

let typedefs2, funs2, classes2 = get_decls defs in

Expand Down Expand Up @@ -823,7 +823,7 @@ let decl_and_run_mode {filename; mode; no_builtins; tcopt} popt =
~init:files_contents
in

let errors, files_info, _ =
let errors, files_info =
parse_name_and_decl popt files_contents_with_builtins tcopt in

handle_mode mode filename tcopt popt files_contents files_info
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/naming/namingGlobal.ml
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ let ndecl_file_fast fn ~funs ~classes ~typedefs ~consts =
let ndecl_file popt fn
{ FileInfo.file_mode = _; funs; classes; typedefs; consts;
comments = _; hash = _} =
let errors, _, _ = Errors.do_with_context fn Errors.Naming begin fun () ->
let errors, _ = Errors.do_with_context fn Errors.Naming begin fun () ->
dn ("Naming decl: "^Relative_path.to_absolute fn);
make_env popt ~funs ~classes ~typedefs ~consts
end in
Expand Down
2 changes: 1 addition & 1 deletion hphp/hack/src/parser/parsing_service.ml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ let legacy_php_file_info = ref (fun fn ->
*)
let process_parse_result
?(ide = false) ~quick (acc, errorl, error_files) fn res popt =
let errorl', {Parser_hack.file_mode; comments; ast; content; is_hh_file = _}, _ = res in
let errorl', {Parser_hack.file_mode; comments; ast; content; is_hh_file = _} = res in
let ast =
if (Relative_path.prefix fn = Relative_path.Hhi)
&& ParserOptions.deregister_php_stdlib popt
Expand Down
5 changes: 3 additions & 2 deletions hphp/hack/src/server/serverConvert.ml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ let print_patch filename tcopt (line, kind, type_) =
(*****************************************************************************)

let add_file env fn =
let failed_parsing = Relative_path.Set.add env.ServerEnv.failed_parsing fn in
{ env with ServerEnv.failed_parsing = failed_parsing }
let disk_needs_parsing =
Relative_path.Set.add env.ServerEnv.disk_needs_parsing fn in
{ env with ServerEnv.disk_needs_parsing = disk_needs_parsing }

(* Maps filenames to the contents of those files, split into lines. Each line
* is numbered with the line number of the *original* line in the file, before
Expand Down
24 changes: 10 additions & 14 deletions hphp/hack/src/server/serverEnv.ml
Original file line number Diff line number Diff line change
Expand Up @@ -69,22 +69,21 @@ type env = {
files_info : FileInfo.t Relative_path.Map.t;
tcopt : TypecheckerOptions.t;
popt : ParserOptions.t;
errorl : Errors.t;
(* Sets of files that were known to GENERATE errors in corresponding phases.
* Note that this is different from HAVING errors - it's possible for
* checking of A to generate error in B - in this case failed_check
* should contain A, not B.
(* Errors are indexed by files that were known to GENERATE errors in
* corresponding phases. Note that this is different from HAVING errors -
* it's possible for checking of A to generate error in B - in this case
* Errors.get_failed_files Typing should contain A, not B.
* Conversly, if declaring A will require declaring B, we should put
* B in failed_decl. Same if checking A will cause declaring B (via lazy
* B in failed decl. Same if checking A will cause declaring B (via lazy
* decl).
*
* During recheck, we add those files to the set of files to reanalyze
* at each stage in order to regenerate their error lists. So those
* failed_ sets are the main piece of mutable state that incremental mode
* needs to maintain - ServerEnv.errorl is more of a cache, and should
* always be possible to be regenerated based on those sets.
*
* failed_naming is also used as kind of a dependency tracking mechanism:
* needs to maintain - the errors themselves are more of a cache, and should
* always be possible to be regenerated based on those sets. *)
errorl : Errors.t;
(* failed_naming is used as kind of a dependency tracking mechanism:
* if files A.php and B.php both define class C, then those files are
* mutually depending on each other (edit to one might resolve naming
* ambiguity and change the interpretation of the other). Both of those
Expand All @@ -93,10 +92,7 @@ type env = {
*
* See test_naming_errors.ml and test_failed_naming.ml
*)
failed_parsing : Relative_path.Set.t;
failed_naming : Relative_path.Set.t;
failed_decl : Relative_path.Set.t;
failed_check : Relative_path.Set.t;
failed_naming : Relative_path.Set.t;
persistent_client : ClientProvider.client option;
(* Whether last received IDE command was IDE_IDLE *)
ide_idle : bool;
Expand Down
5 changes: 1 addition & 4 deletions hphp/hack/src/server/serverEnvBuild.ml
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,7 @@ let make_env config =
popt = ServerConfig.parser_options config;
files_info = Relative_path.Map.empty;
errorl = Errors.empty;
failed_parsing = Relative_path.Set.empty;
failed_naming = Relative_path.Set.empty;
failed_decl = Relative_path.Set.empty;
failed_check = Relative_path.Set.empty;
failed_naming = Relative_path.Set.empty;
persistent_client = None;
ide_idle = false;
last_command_time = 0.0;
Expand Down
Loading

0 comments on commit 8b1b511

Please sign in to comment.