From 9ce3dd0177871a37e9fde90a43a3797d3ff29b27 Mon Sep 17 00:00:00 2001 From: Millie Chen Date: Tue, 21 Sep 2021 16:43:27 -0700 Subject: [PATCH] Remove duplicate code in compile.rs Summary: Previously `hhbc_by_ref_compile:from_text_` and `hhbc_by_ref_compile:hhas_from_text_` use the same block of code to generate an hhas program, except the former also prints the program and the latter simply returns the generated program. This diff factors out the common bits to minimize duplicate code. Reviewed By: shayne-fletcher Differential Revision: D31072068 fbshipit-source-id: 4636aa75a39db21e644f87cbfdfc7ed3f2ea6535 --- hphp/hack/src/hhbc/compile_ffi.rs | 2 +- hphp/hack/src/hhbc/hhbc_by_ref/compile.rs | 208 +++++++++++++----------------- 2 files changed, 89 insertions(+), 121 deletions(-) diff --git a/hphp/hack/src/hhbc/compile_ffi.rs b/hphp/hack/src/hhbc/compile_ffi.rs index af04dc903ec..9713077cda4 100644 --- a/hphp/hack/src/hhbc/compile_ffi.rs +++ b/hphp/hack/src/hhbc/compile_ffi.rs @@ -340,7 +340,7 @@ unsafe extern "C" fn hackc_compile_hhas_from_text_cpp_ffi( ) }; match compile_result { - Ok(hhas_prog) => Ok(Box::into_raw(Box::new(hhas_prog))), + Ok((hhas_prog, _)) => Ok(Box::into_raw(Box::new(hhas_prog))), Err(e) => Err(anyhow!("{}", e)), } }, diff --git a/hphp/hack/src/hhbc/hhbc_by_ref/compile.rs b/hphp/hack/src/hhbc/hhbc_by_ref/compile.rs index 072b05a0952..0cd80ce0f48 100644 --- a/hphp/hack/src/hhbc/hhbc_by_ref/compile.rs +++ b/hphp/hack/src/hhbc/hhbc_by_ref/compile.rs @@ -282,7 +282,7 @@ impl> NativeEnv { /// (this avoids the need to read Options from OCaml, as /// they can be simply returned as NaNs to signal that /// they should _not_ be passed back as JSON to HHVM process) -#[derive(Debug, ToOcamlRep)] +#[derive(Debug, Default, ToOcamlRep)] pub struct Profile { pub parsing_t: f64, pub codegen_t: f64, @@ -359,53 +359,9 @@ where W: Write, W::Error: Send + Sync + 'static, // required by anyhow::Error { - let opts = match native_env { - None => Options::from_configs(&env.config_jsons, &env.config_list) - .map_err(anyhow::Error::msg)?, - Some(native_env) => NativeEnv::to_options(&native_env), - }; - - let mut emitter = Emitter::new( - opts, - env.flags.contains(EnvFlags::IS_SYSTEMLIB), - env.flags.contains(EnvFlags::FOR_DEBUGGER_EVAL), - decl_provider, - ); - - let namespace_env = RcOc::new(NamespaceEnv::empty( - emitter.options().hhvm.aliased_namespaces_cloned().collect(), - true, /* is_codegen */ - emitter - .options() - .hhvm - .hack_lang - .flags - .contains(LangFlags::DISABLE_XHP_ELEMENT_MANGLING), - )); - - let (mut parse_result, parsing_t) = time(|| { - parse_file( - emitter.options(), - stack_limit, - source_text, - !env.flags.contains(EnvFlags::DISABLE_TOPLEVEL_ELABORATION), - RcOc::clone(&namespace_env), - env.flags.contains(EnvFlags::IS_SYSTEMLIB), - ) - }); - let log_extern_compiler_perf = emitter.options().log_extern_compiler_perf(); - - let (program, codegen_t) = match &mut parse_result { - Either::Right(ast) => { - elaborate_namespaces_visitor::elaborate_program(RcOc::clone(&namespace_env), ast); - let e = &mut emitter; - time(move || rewrite_and_emit(alloc, e, &env, namespace_env, ast)) - } - Either::Left((pos, msg, is_runtime_error)) => { - time(|| emit_fatal(alloc, *is_runtime_error, pos, msg)) - } - }; - let program = program.map_err(|e| anyhow!("Unhandled Emitter error: {}", e))?; + let mut emitter = create_emitter(env, native_env, decl_provider)?; + let (program, profile) = + emit_prog_from_text(alloc, &mut emitter, env, stack_limit, source_text)?; let (print_result, printing_t) = time(|| { print_program( @@ -421,15 +377,10 @@ where }); print_result?; - if log_extern_compiler_perf { - Ok(Some(Profile { - parsing_t, - codegen_t, - printing_t, - })) - } else { - Ok(None) - } + Ok(profile.map(|mut prof| { + prof.printing_t = printing_t; + prof + })) } fn rewrite_and_emit<'p, 'arena, 'decl, D: DeclProvider<'decl>, S: AsRef>( @@ -444,7 +395,7 @@ fn rewrite_and_emit<'p, 'arena, 'decl, D: DeclProvider<'decl>, S: AsRef>( match result { Ok(()) => { // Rewrite ok, now emit. - emit(alloc, emitter, &env, namespace_env, ast) + emit_prog_from_ast(alloc, emitter, &env, namespace_env, ast) } Err(Error::IncludeTimeFatalException(op, pos, msg)) => { emit_program::emit_fatal_program(alloc, op, &pos, msg) @@ -469,7 +420,7 @@ pub fn hhas_from_text<'arena, S: AsRef>( text: &[u8], ) -> anyhow::Result> { let source_text = SourceText::make(RcOc::new(env.filepath.clone()), text); - hhas_from_text_(alloc, env, stack_limit, source_text, None, NoDeclProvider) + Ok(hhas_from_text_(alloc, env, stack_limit, source_text, None, NoDeclProvider)?.0) } pub fn hhas_from_text_<'arena, 'decl, S: AsRef, D: DeclProvider<'decl>>( @@ -479,53 +430,9 @@ pub fn hhas_from_text_<'arena, 'decl, S: AsRef, D: DeclProvider<'decl>>( source_text: SourceText, native_env: Option<&NativeEnv>, decl_provider: D, -) -> anyhow::Result> { - let opts = match native_env { - None => Options::from_configs(&env.config_jsons, &env.config_list) - .map_err(anyhow::Error::msg)?, - Some(native_env) => NativeEnv::to_options(&native_env), - }; - - let mut emitter = Emitter::new( - opts, - env.flags.contains(EnvFlags::IS_SYSTEMLIB), - env.flags.contains(EnvFlags::FOR_DEBUGGER_EVAL), - decl_provider, - ); - - let namespace_env = RcOc::new(NamespaceEnv::empty( - emitter.options().hhvm.aliased_namespaces_cloned().collect(), - true, /* is_codegen */ - emitter - .options() - .hhvm - .hack_lang - .flags - .contains(LangFlags::DISABLE_XHP_ELEMENT_MANGLING), - )); - - let (parse_result, _) = time(|| { - parse_file( - emitter.options(), - stack_limit, - source_text, - !env.flags.contains(EnvFlags::DISABLE_TOPLEVEL_ELABORATION), - RcOc::clone(&namespace_env), - env.flags.contains(EnvFlags::IS_SYSTEMLIB), - ) - }); - - let (program, _) = match parse_result { - Either::Right(mut ast) => { - elaborate_namespaces_visitor::elaborate_program(RcOc::clone(&namespace_env), &mut ast); - let e = &mut emitter; - time(move || rewrite_and_emit(alloc, e, &env, namespace_env, &mut ast)) - } - Either::Left((pos, msg, is_runtime_error)) => { - time(|| emit_fatal(alloc, is_runtime_error, &pos, msg)) - } - }; - program.map_err(|e| anyhow!("Unhandled Emitter error: {}", e)) +) -> anyhow::Result<(HhasProgram<'arena>, Option)> { + let mut emitter = create_emitter(env, native_env, decl_provider)?; + emit_prog_from_text(alloc, &mut emitter, env, stack_limit, source_text) } pub fn hhas_to_string<'decl, W: std::fmt::Write, S: AsRef, D: DeclProvider<'decl>>( @@ -535,19 +442,7 @@ pub fn hhas_to_string<'decl, W: std::fmt::Write, S: AsRef, D: DeclProvider< program: &HhasProgram, decl_provider: D, ) -> anyhow::Result<()> { - let opts = match native_env { - None => Options::from_configs(&env.config_jsons, &env.config_list) - .map_err(anyhow::Error::msg)?, - Some(native_env) => NativeEnv::to_options(&native_env), - }; - - let mut emitter = Emitter::new( - opts, - env.flags.contains(EnvFlags::IS_SYSTEMLIB), - env.flags.contains(EnvFlags::FOR_DEBUGGER_EVAL), - decl_provider, - ); - + let mut emitter = create_emitter(env, native_env, decl_provider)?; let (print_result, _) = time(|| { print_program( &mut Context::new( @@ -563,7 +458,7 @@ pub fn hhas_to_string<'decl, W: std::fmt::Write, S: AsRef, D: DeclProvider< print_result.map_err(|e| anyhow!("{}", e)) } -fn emit<'p, 'arena, 'decl, D: DeclProvider<'decl>, S: AsRef>( +fn emit_prog_from_ast<'p, 'arena, 'decl, D: DeclProvider<'decl>, S: AsRef>( alloc: &'arena bumpalo::Bump, emitter: &mut Emitter<'arena, 'decl, D>, env: &Env, @@ -598,6 +493,61 @@ fn emit<'p, 'arena, 'decl, D: DeclProvider<'decl>, S: AsRef>( emit_program(alloc, emitter, flags, namespace, ast) } +fn emit_prog_from_text<'arena, 'decl, S: AsRef, D: DeclProvider<'decl>>( + alloc: &'arena bumpalo::Bump, + emitter: &mut Emitter<'arena, 'decl, D>, + env: &Env, + stack_limit: &StackLimit, + source_text: SourceText, +) -> anyhow::Result<(HhasProgram<'arena>, Option)> { + let log_extern_compiler_perf = emitter.options().log_extern_compiler_perf(); + + let namespace_env = RcOc::new(NamespaceEnv::empty( + emitter.options().hhvm.aliased_namespaces_cloned().collect(), + true, /* is_codegen */ + emitter + .options() + .hhvm + .hack_lang + .flags + .contains(LangFlags::DISABLE_XHP_ELEMENT_MANGLING), + )); + + let (parse_result, parsing_t) = time(|| { + parse_file( + emitter.options(), + stack_limit, + source_text, + !env.flags.contains(EnvFlags::DISABLE_TOPLEVEL_ELABORATION), + RcOc::clone(&namespace_env), + env.flags.contains(EnvFlags::IS_SYSTEMLIB), + ) + }); + + let (program, codegen_t) = match parse_result { + Either::Right(mut ast) => { + elaborate_namespaces_visitor::elaborate_program(RcOc::clone(&namespace_env), &mut ast); + time(move || rewrite_and_emit(alloc, emitter, &env, namespace_env, &mut ast)) + } + Either::Left((pos, msg, is_runtime_error)) => { + time(|| emit_fatal(alloc, is_runtime_error, &pos, msg)) + } + }; + let profile = if log_extern_compiler_perf { + Some(Profile { + parsing_t, + codegen_t, + ..Default::default() + }) + } else { + None + }; + match program { + Ok(prog) => Ok((prog, profile)), + Err(e) => Err(anyhow!("Unhandled Emitter error: {}", e)), + } +} + fn emit_fatal<'arena>( alloc: &'arena bumpalo::Bump, is_runtime_error: bool, @@ -612,6 +562,24 @@ fn emit_fatal<'arena>( emit_program::emit_fatal_program(alloc, op, pos, msg) } +fn create_emitter<'arena, 'decl, S: AsRef, D: DeclProvider<'decl>>( + env: &Env, + native_env: Option<&NativeEnv>, + decl_provider: D, +) -> anyhow::Result> { + let opts = match native_env { + None => Options::from_configs(&env.config_jsons, &env.config_list) + .map_err(anyhow::Error::msg)?, + Some(native_env) => NativeEnv::to_options(&native_env), + }; + Ok(Emitter::new( + opts, + env.flags.contains(EnvFlags::IS_SYSTEMLIB), + env.flags.contains(EnvFlags::FOR_DEBUGGER_EVAL), + decl_provider, + )) +} + fn create_parser_options(opts: &Options) -> ParserOptions { let hack_lang_flags = |flag| opts.hhvm.hack_lang.flags.contains(flag); let phpism_flags = |flag| opts.phpism_flags.contains(flag); -- 2.11.4.GIT