rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.33k stars 12.58k forks source link

Audit all UNTRACKED options #84232

Open jyn514 opened 3 years ago

jyn514 commented 3 years ago

I know of several bugs due to untracked options that should be tracked (e.g. https://github.com/rust-lang/rust/issues/66955, https://github.com/rust-lang/rust/issues/84225). Maybe we should go through all the untracked options and make sure they should actually be untracked?

All untracked options ``` $ grep '\[UNTRACKED\]' compiler/rustc_session/src/options.rs ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, [UNTRACKED]) => {{}}; // [UNTRACKED] describe_lints: bool [UNTRACKED], search_paths: Vec [UNTRACKED], maybe_sysroot: Option [UNTRACKED], error_format: ErrorOutputType [UNTRACKED], incremental: Option [UNTRACKED], prints: Vec [UNTRACKED], borrowck_mode: BorrowckMode [UNTRACKED], externs: Externs [UNTRACKED], extern_dep_specs: ExternDepSpecs [UNTRACKED], cli_forced_codegen_units: Option [UNTRACKED], cli_forced_thinlto_off: bool [UNTRACKED], remap_path_prefix: Vec<(PathBuf, PathBuf)> [UNTRACKED], json_unused_externs: bool [UNTRACKED], pretty: Option [UNTRACKED], ar: String = (String::new(), parse_string, [UNTRACKED], codegen_units: Option = (None, parse_opt_number, [UNTRACKED], default_linker_libraries: bool = (false, parse_bool, [UNTRACKED], extra_filename: String = (String::new(), parse_string, [UNTRACKED], incremental: Option = (None, parse_opt_string, [UNTRACKED], link_arg: (/* redirected to link_args */) = ((), parse_string_push, [UNTRACKED], link_args: Vec = (Vec::new(), parse_list, [UNTRACKED], link_dead_code: Option = (None, parse_opt_bool, [UNTRACKED], link_self_contained: Option = (None, parse_opt_bool, [UNTRACKED], linker: Option = (None, parse_opt_pathbuf, [UNTRACKED], linker_flavor: Option = (None, parse_linker_flavor, [UNTRACKED], no_stack_check: bool = (false, parse_no_flag, [UNTRACKED], remark: Passes = (Passes::Some(Vec::new()), parse_passes, [UNTRACKED], rpath: bool = (false, parse_bool, [UNTRACKED], save_temps: bool = (false, parse_bool, [UNTRACKED], ast_json: bool = (false, parse_bool, [UNTRACKED], ast_json_noexpand: bool = (false, parse_bool, [UNTRACKED], borrowck: String = ("migrate".to_string(), parse_string, [UNTRACKED], deduplicate_diagnostics: bool = (true, parse_bool, [UNTRACKED], dep_tasks: bool = (false, parse_bool, [UNTRACKED], dont_buffer_diagnostics: bool = (false, parse_bool, [UNTRACKED], dump_dep_graph: bool = (false, parse_bool, [UNTRACKED], dump_mir: Option = (None, parse_opt_string, [UNTRACKED], dump_mir_dataflow: bool = (false, parse_bool, [UNTRACKED], dump_mir_dir: String = ("mir_dump".to_string(), parse_string, [UNTRACKED], dump_mir_exclude_pass_number: bool = (false, parse_bool, [UNTRACKED], dump_mir_graphviz: bool = (false, parse_bool, [UNTRACKED], dump_mir_spanview: Option = (None, parse_mir_spanview, [UNTRACKED], emit_future_incompat_report: bool = (false, parse_bool, [UNTRACKED], emit_stack_sizes: bool = (false, parse_bool, [UNTRACKED], graphviz_dark_mode: bool = (false, parse_bool, [UNTRACKED], graphviz_font: String = ("Courier, monospace".to_string(), parse_string, [UNTRACKED], hir_stats: bool = (false, parse_bool, [UNTRACKED], identify_regions: bool = (false, parse_bool, [UNTRACKED], incremental_ignore_spans: bool = (false, parse_bool, [UNTRACKED], incremental_info: bool = (false, parse_bool, [UNTRACKED], incremental_verify_ich: bool = (false, parse_bool, [UNTRACKED], input_stats: bool = (false, parse_bool, [UNTRACKED], keep_hygiene_data: bool = (false, parse_bool, [UNTRACKED], link_native_libraries: bool = (true, parse_bool, [UNTRACKED], llvm_time_trace: bool = (false, parse_bool, [UNTRACKED], ls: bool = (false, parse_bool, [UNTRACKED], macro_backtrace: bool = (false, parse_bool, [UNTRACKED], meta_stats: bool = (false, parse_bool, [UNTRACKED], nll_facts: bool = (false, parse_bool, [UNTRACKED], nll_facts_dir: String = ("nll-facts".to_string(), parse_string, [UNTRACKED], no_analysis: bool = (false, parse_no_flag, [UNTRACKED], no_interleave_lints: bool = (false, parse_no_flag, [UNTRACKED], no_leak_check: bool = (false, parse_no_flag, [UNTRACKED], no_parallel_llvm: bool = (false, parse_no_flag, [UNTRACKED], parse_only: bool = (false, parse_bool, [UNTRACKED], perf_stats: bool = (false, parse_bool, [UNTRACKED], pre_link_arg: (/* redirected to pre_link_args */) = ((), parse_string_push, [UNTRACKED], pre_link_args: Vec = (Vec::new(), parse_list, [UNTRACKED], print_link_args: bool = (false, parse_bool, [UNTRACKED], print_llvm_passes: bool = (false, parse_bool, [UNTRACKED], print_mono_items: Option = (None, parse_opt_string, [UNTRACKED], print_type_sizes: bool = (false, parse_bool, [UNTRACKED], proc_macro_backtrace: bool = (false, parse_bool, [UNTRACKED], query_dep_graph: bool = (false, parse_bool, [UNTRACKED], query_stats: bool = (false, parse_bool, [UNTRACKED], save_analysis: bool = (false, parse_bool, [UNTRACKED], parse_switch_with_opt_path, [UNTRACKED], self_profile_events: Option> = (None, parse_opt_comma_list, [UNTRACKED], span_debug: bool = (false, parse_bool, [UNTRACKED], span_free_formats: bool = (false, parse_bool, [UNTRACKED], strip: Strip = (Strip::None, parse_strip, [UNTRACKED], split_dwarf_inlining: bool = (true, parse_bool, [UNTRACKED], terminal_width: Option = (None, parse_opt_number, [UNTRACKED], threads: usize = (1, parse_threads, [UNTRACKED], time: bool = (false, parse_bool, [UNTRACKED], time_llvm_passes: bool = (false, parse_bool, [UNTRACKED], time_passes: bool = (false, parse_bool, [UNTRACKED], trace_macros: bool = (false, parse_bool, [UNTRACKED], trim_diagnostic_paths: bool = (true, parse_bool, [UNTRACKED], ui_testing: bool = (false, parse_bool, [UNTRACKED], unpretty: Option = (None, parse_unpretty, [UNTRACKED], unstable_options: bool = (false, parse_bool, [UNTRACKED], validate_mir: bool = (false, parse_bool, [UNTRACKED], verbose: bool = (false, parse_bool, [UNTRACKED], ```

cc @Aaron1011

jyn514 commented 3 years ago

Just off the top of my head, codegen_units, incremental_ignore_spans, borrowck, and maybe_sysroot all look suspicious.

Mark-Simulacrum commented 3 years ago

I worry that a one time audit may help but ultimately not fix the long tail as changes are added. Maybe we should default to tracked, updating all existing options, and then relax if we can definitely trust it to not need to be tracked?

jyn514 commented 3 years ago

I wonder if we could automate at least some of this by doing incremental builds with a clean cache with and without the option, then checking if the cache is the same. I don't know if there's an easy way to compare two caches; maybe just a binary comparison is good enough?

jyn514 commented 3 years ago

A simpler but less effective version of that is to only compare the final binary.