Kill source mapping option across stack

Summary:
This option is explicitly set to true at every entry point to HackC,
as well as in tests, but it's true by default in HHVM.
It's also an outlier because it has 2 aliases when used as CLI argument:
- hack.compiler.sourcemapping
- eval.disassemblersourcemapping

neither of which matches its JSON key: eval.disassembler_source_mapping.

All of the abovee adds significant complexity, so kill it altogether.

Reviewed By: shiqicao, alexeyt

Differential Revision: D20295585

fbshipit-source-id: bed0040227143b6df122e2f11545bde909ace70e

Author: losvald

hphp/hack/src/hhbc/emit_pos.ml

@@ -11,10 +11,7 @@ open Instruction_sequence
 open Hhbc_ast
 
 let emit_pos pos =
-  if
-    Hhbc_options.source_mapping !Hhbc_options.compiler_options
-    && pos <> Pos.none
-  then
+  if pos <> Pos.none then
     let (line_begin, line_end, col_begin, col_end) =
       Pos.info_pos_extended pos
     in

hphp/hack/src/hhbc/emit_pos.rs

@@ -5,17 +5,11 @@
 
 use env::emitter::Emitter;
 use instruction_sequence_rust::InstrSeq;
-use options::EvalFlags;
 use oxidized::pos::Pos;
 use std::convert::TryInto;
 
 pub fn emit_pos(emitter: &Emitter, pos: &Pos) -> InstrSeq {
-    if emitter
-        .options()
-        .eval_flags
-        .contains(EvalFlags::DISASSEMBLER_SOURCE_MAPPING)
-        && !pos.is_none()
-    {
+    if !pos.is_none() {
         let (line_begin, line_end, col_begin, col_end) = pos.info_pos_extended();
         InstrSeq::make_srcloc(
             line_begin.try_into().unwrap(),

hphp/hack/src/hhbc/hhbc_hhas.ml

@@ -1594,8 +1594,7 @@ let add_fun_def buf fun_def =
   if Hhbc_options.emit_generics_ub !Hhbc_options.compiler_options then
     Acc.add buf (string_of_upper_bounds function_upper_bounds);
   Acc.add buf (function_attributes fun_def);
-  if Hhbc_options.source_mapping !Hhbc_options.compiler_options then
-    Acc.add buf (string_of_span function_span ^ " ");
+  Acc.add buf (string_of_span function_span ^ " ");
   Acc.add buf (string_of_type_info_option function_return_type);
   Acc.add buf (Hhbc_id.Function.to_raw_string function_name);
   Acc.add buf (string_of_params env function_params);
@@ -1734,8 +1733,7 @@ let add_method_def buf method_def =
     Acc.add buf (string_of_upper_bounds method_upper_bounds)
   );
   Acc.add buf (method_attributes method_def);
-  if Hhbc_options.source_mapping !Hhbc_options.compiler_options then
-    Acc.add buf (string_of_span method_span ^ " ");
+  Acc.add buf (string_of_span method_span ^ " ");
   Acc.add buf (string_of_type_info_option method_return_type);
   Acc.add buf (Hhbc_id.Method.to_raw_string method_name);
   Acc.add buf (string_of_params env method_params);
@@ -2102,8 +2100,7 @@ let add_class_def buf class_def =
     Acc.add buf (string_of_upper_bounds (Hhas_class.upper_bounds class_def));
   Acc.add buf (class_special_attributes class_def);
   Acc.add buf (Hhbc_id.Class.to_raw_string class_name);
-  if Hhbc_options.source_mapping !Hhbc_options.compiler_options then
-    Acc.add buf (" " ^ string_of_span (Hhas_class.span class_def));
+  Acc.add buf (" " ^ string_of_span (Hhas_class.span class_def));
   add_extends
     buf
     (Option.map ~f:Hhbc_id.Class.to_raw_string (Hhas_class.base class_def));
@@ -2168,8 +2165,7 @@ let add_data_region buf adata =
 
 let add_main buf body =
   Acc.add buf ".main ";
-  if Hhbc_options.source_mapping !Hhbc_options.compiler_options then
-    Acc.add buf "(1,1) ";
+  Acc.add buf "(1,1) ";
   Acc.add buf "{";
   add_body buf 2 body;
   Acc.add buf "}\n"

hphp/hack/src/hhbc/hhbc_options.ml

@@ -16,7 +16,6 @@ type t = {
   option_optimize_null_checks: bool;
   option_max_array_elem_size_on_the_stack: int;
   option_aliased_namespaces: (string * string) list;
-  option_source_mapping: bool;
   option_relabel: bool;
   option_php7_uvs: bool;
   option_php7_ltr_assign: bool;
@@ -70,7 +69,6 @@ let default =
     option_optimize_null_checks = false;
     option_max_array_elem_size_on_the_stack = 64;
     option_aliased_namespaces = [];
-    option_source_mapping = false;
     option_php7_uvs = false;
     option_php7_ltr_assign = false;
     (* If true, then renumber labels after generating code for a method
@@ -130,8 +128,6 @@ let max_array_elem_size_on_the_stack o =
 
 let aliased_namespaces o = o.option_aliased_namespaces
 
-let source_mapping o = o.option_source_mapping
-
 let relabel o = o.option_relabel
 
 let enable_uniform_variable_syntax o = o.option_php7_uvs
@@ -254,7 +250,6 @@ let to_string o =
       Printf.sprintf "max_array_elem_size_on_the_stack: %d"
       @@ max_array_elem_size_on_the_stack o;
       Printf.sprintf "aliased_namespaces: %s" aliased_namespaces_str;
-      Printf.sprintf "source_mapping: %B" @@ source_mapping o;
       Printf.sprintf "relabel: %B" @@ relabel o;
       Printf.sprintf "enable_uniform_variable_syntax: %B"
       @@ enable_uniform_variable_syntax o;
@@ -335,10 +330,6 @@ let set_option options name value =
     { options with option_constant_folding = as_bool value }
   | "hack.compiler.optimizenullcheck" ->
     { options with option_optimize_null_checks = as_bool value }
-  (* Keep both for backwards compatibility until next release *)
-  | "hack.compiler.sourcemapping"
-  | "eval.disassemblersourcemapping" ->
-    { options with option_source_mapping = as_bool value }
   | "hhvm.php7.ltr_assign" ->
     { options with option_php7_ltr_assign = as_bool value }
   | "hhvm.php7.uvs" -> { options with option_php7_uvs = as_bool value }
@@ -502,8 +493,6 @@ let value_setters =
     @@ fun opts v -> { opts with option_constant_folding = v = 1 } );
     ( set_value "hack.compiler.optimize_null_checks" get_value_from_config_int
     @@ fun opts v -> { opts with option_optimize_null_checks = v = 1 } );
-    ( set_value "eval.disassembler_source_mapping" get_value_from_config_int
-    @@ fun opts v -> { opts with option_source_mapping = v = 1 } );
     ( set_value "hhvm.php7.uvs" get_value_from_config_int @@ fun opts v ->
       { opts with option_php7_uvs = v = 1 } );
     ( set_value "hhvm.php7.ltr_assign" get_value_from_config_int @@ fun opts v ->

hphp/hack/src/hhbc/options.rs

@@ -137,15 +137,6 @@ impl<T> Arg<T> {
 
 // group options by JSON config prefix to avoid error-prone repetition & boilerplate in SerDe
 
-// TODO move this "lonely wolf" to group "hack.compiler." (as hinted in D7057454);
-// however, this is a breaking change for stuff that passes this option as JSON key
-prefixed_flags!(EvalFlags, "eval.", DISASSEMBLER_SOURCE_MAPPING,);
-impl Default for EvalFlags {
-    fn default() -> EvalFlags {
-        EvalFlags::empty()
-    }
-}
-
 prefixed_flags!(
     CompilerFlags,
     "hack.compiler.",
@@ -289,13 +280,6 @@ pub struct Options {
     #[serde(default)]
     pub doc_root: Arg<String>,
 
-    #[serde(
-        flatten,
-        serialize_with = "serialize_flags",
-        deserialize_with = "deserialize_flags"
-    )]
-    pub eval_flags: EvalFlags,
-
     #[serde(
         flatten,
         serialize_with = "serialize_flags",
@@ -347,7 +331,6 @@ impl Default for Options {
         Options {
             max_array_elem_size_on_the_stack: defaults::max_array_elem_size_on_the_stack(),
             hack_compiler_flags: CompilerFlags::default(),
-            eval_flags: EvalFlags::default(),
             hhvm: Hhvm::default(),
             phpism_flags: PhpismFlags::default(),
             php7_flags: Php7Flags::default(),
@@ -441,11 +424,6 @@ impl Options {
         opts.map_err(|e| e.to_string())
     }
 
-    pub fn source_map(&self) -> bool {
-        self.eval_flags
-            .contains(EvalFlags::DISASSEMBLER_SOURCE_MAPPING)
-    }
-
     pub fn array_provenance(&self) -> bool {
         self.hhvm.flags.contains(HhvmFlags::ARRAY_PROVENANCE)
     }
@@ -962,46 +940,6 @@ mod tests {
         }))
         .expect("boolish-parsing logic wrongly triggered");
     }
-
-    #[test]
-    fn test_major_outlier_source_mapping_serde() {
-        use serde::ser::Serialize;
-        fn mk_source_mapping<T: Serialize>(v: T) -> Json {
-            json!({
-                "eval.disassembler_source_mapping": {
-                    "global_value": v
-                }
-            })
-        }
-        fn serialize_source_mapping(opts: Options) -> Json {
-            let mut j = serde_json::to_value(opts).unwrap();
-            // remove everything from Options JSON except this key
-            const KEY: &str = "eval.disassembler_source_mapping";
-            let m = j.as_object_mut().unwrap();
-            let sm = m.remove(KEY);
-            m.clear();
-            sm.map(|val| m.insert(KEY.to_owned(), val));
-            j
-        }
-        fn test<T: Serialize + std::fmt::Debug>(exp_val: bool, val: T) {
-            let j = mk_source_mapping(dbg!(val));
-            let opts: Options = serde_json::value::from_value(j).unwrap();
-            assert_eq!(
-                exp_val,
-                opts.eval_flags
-                    .contains(EvalFlags::DISASSEMBLER_SOURCE_MAPPING)
-            );
-            let j_act = serialize_source_mapping(opts);
-            let j_exp = mk_source_mapping(exp_val);
-            assert_eq!(j_exp, j_act);
-        }
-        test(true, true);
-        test(true, 1);
-        test(true, "true");
-        test(false, 0);
-        test(false, false);
-        test(false, "false");
-    }
 }
 
 // boilerplate code that could eventually be avoided via procedural macros
@@ -1010,7 +948,6 @@ bitflags! {
     struct Flags: u64 {
         const CONSTANT_FOLDING = 1 << 0;
         const OPTIMIZE_NULL_CHECKS = 1 << 1;
-        const DISASSEMBLER_SOURCE_MAPPING = 1 << 2;
         const UVS = 1 << 3;
         const LTR_ASSIGN = 1 << 4;
         /// If true, then renumber labels after generating code for a method

hphp/hack/src/hhbc/options_cli.rs

@@ -26,36 +26,33 @@ lazy_static! {
     /// the exceptions are overridden here for config list (-v KEY=VAL).
     pub static ref CANON_BY_ALIAS: HashMap<&'static str, &'static str> =
         impl_CANON_BY_ALIAS!(
-            // group 1: multiple names, neither consistent with JSON field name
-            "hack.compiler.sourcemapping" =>    "eval.disassembler_source_mapping",
-            "eval.disassemblersourcemapping" => "eval.disassembler_source_mapping",
-            // group 2: obtained by removing underscores from JSON field names
+            // group 1: obtained by removing underscores from JSON field names
             "hack.compiler.constantfolding" => "hack.compiler.constant_folding",
             "hack.compiler.optimizenullcheck" => "hack.compiler.optimize_null_checks",
-            // group 3: also different prefix without any obvious rule
+            // group 2: also different prefix without any obvious rule
             "eval.hackarrcompatnotices" => "hhvm.hack_arr_compat_notices",
             "eval.hackarrdvarrs" => "hhvm.hack_arr_dv_arrs",
             "eval.emitfuncpointers" => "hhvm.emit_func_pointers",
             "eval.jitenablerenamefunction" => "hhvm.jit_enable_rename_function",
             "eval.logexterncompilerperf" => "hhvm.log_extern_compiler_perf",
             "eval.enableintrinsicsextension" => "hhvm.enable_intrinsics_extension",
             "eval.emitgenericsub" => "hhvm.emit_generics_ub",
-            // group 4: we could ignore hhvm. part of prefix in deser.
+            // group 3: we could ignore hhvm. part of prefix in deser.
             "hack.lang.disable_lval_as_an_expression" => "hhvm.hack.lang.disable_lval_as_an_expression",
-            // group 5: combination of group 4 & 2
+            // group 4: combination of group 3 & 1
             "hack.lang.phpism.disallowexecutionoperator" => "hhvm.hack.lang.phpism.disallow_execution_operator",
             "hack.lang.phpism.disablenontopleveldeclarations" => "hhvm.hack.lang.phpism.disable_nontoplevel_declarations",
             "hack.lang.phpism.disablestaticclosures" => "hhvm.hack.lang.phpism.disable_static_closures",
             "hack.lang.phpism.disablehaltcompiler" => "hhvm.hack.lang.phpism.disable_halt_compiler",
             "hack.lang.enablecoroutines" => "hhvm.hack.lang.enable_coroutines",
             "hack.lang.enablepocketuniverses" => "hhvm.hack.lang.enable_pocket_universes",
-            // group 6: we could assume "hack." between "hhvm." and "lang."
+            // group 5: we could assume "hack." between "hhvm." and "lang."
             "hhvm.lang.enable_class_level_where_clauses" => "hhvm.hack.lang.enable_class_level_where_clauses",
             "hhvm.lang.disable_legacy_soft_typehints" => "hhvm.hack.lang.disable_legacy_soft_typehints",
             "hhvm.lang.allow_new_attribute_syntax" => "hhvm.hack.lang.allow_new_attribute_syntax",
             "hhvm.lang.disable_legacy_attribute_syntax" => "hhvm.hack.lang.disable_legacy_attribute_syntax",
             "hhvm.lang.disallow_func_ptrs_in_constants" => "hhvm.hack.lang.disallow_func_ptrs_in_constants",
-            // group 7: combination of group 6 & 2
+            // group 6: combination of group 5 & 1
             "hhvm.lang.constdefaultfuncargs" => "hhvm.hack.lang.const_default_func_args",
             "hhvm.lang.abstractstaticprops" => "hhvm.hack.lang.abstract_static_props",
             "hhvm.lang.disableunsetclassconst" => "hhvm.hack.lang.disable_unset_class_const",

hphp/hack/src/hhbc/print.rs

@@ -292,10 +292,8 @@ fn print_fun_def<W: Write>(
         print_upper_bounds(w, &body.upper_bounds)?;
     }
     print_fun_attrs(ctx, w, fun_def)?;
-    if ctx.opts.source_map() {
-        w.write(string_of_span(&fun_def.span))?;
-        w.write(" ")?;
-    }
+    w.write(string_of_span(&fun_def.span))?;
+    w.write(" ")?;
     option(w, &body.return_type_info, print_type_info)?;
     w.write(" ")?;
     w.write(fun_def.name.to_raw_string())?;
@@ -678,9 +676,7 @@ fn print_class_def<W: Write>(
     }
     print_class_special_attributes(ctx, w, class_def)?;
     w.write(class_def.name.to_raw_string())?;
-    if ctx.opts.source_map() {
-        w.write(format!(" {}", string_of_span(&class_def.span)))?;
-    }
+    w.write(format!(" {}", string_of_span(&class_def.span)))?;
     print_extends(w, class_def.base.as_ref().map(|x| x.to_raw_string()))?;
     print_implements(w, &class_def.implements)?;
     w.write(" {")?;
@@ -865,9 +861,7 @@ fn print_file_attributes<W: Write>(
 
 fn print_main<W: Write>(ctx: &mut Context, w: &mut W, body: &HhasBody) -> Result<(), W::Error> {
     w.write(".main ")?;
-    if ctx.opts.source_map() {
-        w.write("(1,1) ")?;
-    }
+    w.write("(1,1) ")?;
     wrap_by_braces(w, |w| {
         ctx.block(w, |c, w| print_body(c, w, body))?;
         newline(w)

hphp/hack/test/pocket_universes/compile/closure.good.php.exp

@@ -6,16 +6,19 @@
 .adata A_0 = """Y:1:{s:8:\"nullable\";b:1;}""";
 .adata A_1 = """v:0:{}""";
 
-.main {
+.main (1,1) {
+  .srcloc 4:7,4:7;
   DefCls 0
   Int 1
   RetC
 }
 
-.class C {
-  .method [public static] <"HH\\void" N > derive(<"TE" "TE" extended_hint type_var> $field, <"HH\\mixed" N > $data) {
+.class C (4,14) {
+  .method [public static] (10,13) <"HH\\void" N > derive(<"TE" "TE" extended_hint type_var> $field, <"HH\\mixed" N > $data) {
     .declvars $_;
+    .srcloc 10:17,13:135;
     VerifyParamType $field
+    .srcloc 12:12,12:16;
     CGetL $data
     SetL _3
     Array @A_0
@@ -28,17 +31,25 @@
   L0:
     PushL _3
     UnsetL _4
+    .srcloc 12:7,12:39;
     SetL $_
+    .srcloc 12:7,12:40;
     PopC
+    .srcloc 13:6,13:6;
     Null
     RetC
   }
-  .method [public static] <"HH\\mixed" N > E##Members() {
+  .method [public static] (5,5) <"HH\\mixed" N > E##Members() {
     .declvars $mems;
+    .srcloc 5:8,5:8;
     Vec @A_1
+    .srcloc 5:8,5:8;
     SetL $mems
+    .srcloc 5:8,5:8;
     PopC
+    .srcloc 5:8,5:8;
     CGetL $mems
+    .srcloc 5:8,5:8;
     RetC
   }
 }