From 9d766ed593f7ab9811324856c03730312d3425e3 Mon Sep 17 00:00:00 2001
From: Mazdak Farrokhzad <twingoow@gmail.com>
Date: Fri, 11 Oct 2019 23:30:58 +0200
Subject: [PATCH] refactor
 session::config::build_session_options_and_crate_config

---
 src/librustc/session/config.rs       | 464 ++++++++++++++++-----------
 src/librustc/session/config/tests.rs |  15 +-
 src/librustc_driver/lib.rs           |   3 +-
 3 files changed, 298 insertions(+), 184 deletions(-)

diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index 25e68e6408de2..5af4460016c53 100644
--- a/src/librustc/session/config.rs
+++ b/src/librustc/session/config.rs
@@ -2035,11 +2035,7 @@ pub fn parse_error_format(
     return error_format;
 }
 
-pub fn build_session_options_and_crate_config(
-    matches: &getopts::Matches,
-) -> (Options, FxHashSet<(String, Option<String>)>) {
-    let color = parse_color(matches);
-
+fn parse_crate_edition(matches: &getopts::Matches) -> Edition {
     let edition = match matches.opt_str("edition") {
         Some(arg) => Edition::from_str(&arg).unwrap_or_else(|_|
             early_error(
@@ -2066,19 +2062,14 @@ pub fn build_session_options_and_crate_config(
         )
     }
 
-    let (json_rendered, json_artifact_notifications) = parse_json(matches);
-
-    let error_format = parse_error_format(matches, color, json_rendered);
-
-    let unparsed_crate_types = matches.opt_strs("crate-type");
-    let crate_types = parse_crate_types_from_list(unparsed_crate_types)
-        .unwrap_or_else(|e| early_error(error_format, &e[..]));
-
-
-    let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format);
-
-    let mut debugging_opts = build_debugging_options(matches, error_format);
+    edition
+}
 
+fn check_debug_option_stability(
+    debugging_opts: &DebuggingOptions,
+    error_format: ErrorOutputType,
+    json_rendered: HumanReadableErrorType,
+) {
     if !debugging_opts.unstable_options {
         if let ErrorOutputType::Json { pretty: true, json_rendered } = error_format {
             early_error(
@@ -2094,7 +2085,13 @@ pub fn build_session_options_and_crate_config(
             );
         }
     }
+}
 
+fn parse_output_types(
+    debugging_opts: &DebuggingOptions,
+    matches: &getopts::Matches,
+    error_format: ErrorOutputType,
+) -> OutputTypes {
     let mut output_types = BTreeMap::new();
     if !debugging_opts.parse_only {
         for list in matches.opt_strs("emit") {
@@ -2119,14 +2116,19 @@ pub fn build_session_options_and_crate_config(
     if output_types.is_empty() {
         output_types.insert(OutputType::Exe, None);
     }
+    OutputTypes(output_types)
+}
 
-    let mut cg = build_codegen_options(matches, error_format);
-    let mut codegen_units = cg.codegen_units;
+fn should_override_cgus_and_disable_thinlto(
+    output_types: &OutputTypes,
+    matches: &getopts::Matches,
+    error_format: ErrorOutputType,
+    mut codegen_units: Option<usize>,
+) -> (bool, Option<usize>) {
     let mut disable_thinlto = false;
-
     // Issue #30063: if user requests LLVM-related output to one
     // particular path, disable codegen-units.
-    let incompatible: Vec<_> = output_types
+    let incompatible: Vec<_> = output_types.0
         .iter()
         .map(|ot_path| ot_path.0)
         .filter(|ot| !ot.is_compatible_with_codegen_units_and_single_output_file())
@@ -2158,29 +2160,39 @@ pub fn build_session_options_and_crate_config(
         }
     }
 
-    if debugging_opts.threads == 0 {
+    if codegen_units == Some(0) {
         early_error(
             error_format,
-            "value for threads must be a positive non-zero integer",
+            "value for codegen units must be a positive non-zero integer",
         );
     }
 
-    if debugging_opts.threads > 1 && debugging_opts.fuel.is_some() {
+    (disable_thinlto, codegen_units)
+}
+
+fn check_thread_count(debugging_opts: &DebuggingOptions, error_format: ErrorOutputType) {
+    if debugging_opts.threads == 0 {
         early_error(
             error_format,
-            "optimization fuel is incompatible with multiple threads",
+            "value for threads must be a positive non-zero integer",
         );
     }
 
-    if codegen_units == Some(0) {
+    if debugging_opts.threads > 1 && debugging_opts.fuel.is_some() {
         early_error(
             error_format,
-            "value for codegen units must be a positive non-zero integer",
+            "optimization fuel is incompatible with multiple threads",
         );
     }
+}
 
-    let incremental = match (&debugging_opts.incremental, &cg.incremental) {
-        (&Some(ref path1), &Some(ref path2)) => {
+fn select_incremental_path(
+    debugging_opts: &DebuggingOptions,
+    cg: &CodegenOptions,
+    error_format: ErrorOutputType,
+) -> Option<PathBuf> {
+    match (&debugging_opts.incremental, &cg.incremental) {
+        (Some(path1), Some(path2)) => {
             if path1 != path2 {
                 early_error(
                     error_format,
@@ -2194,25 +2206,19 @@ pub fn build_session_options_and_crate_config(
                 Some(path1)
             }
         }
-        (&Some(ref path), &None) => Some(path),
-        (&None, &Some(ref path)) => Some(path),
-        (&None, &None) => None,
-    }.map(|m| PathBuf::from(m));
-
-    if debugging_opts.profile && incremental.is_some() {
-        early_error(
-            error_format,
-            "can't instrument with gcov profiling when compiling incrementally",
-        );
-    }
-
-    if cg.profile_generate.enabled() && cg.profile_use.is_some() {
-        early_error(
-            error_format,
-            "options `-C profile-generate` and `-C profile-use` are exclusive",
-        );
-    }
+        (Some(path), None) => Some(path),
+        (None, Some(path)) => Some(path),
+        (None, None) => None,
+    }.map(|m| PathBuf::from(m))
+}
 
+fn collect_print_requests(
+    cg: &mut CodegenOptions,
+    dopts: &mut DebuggingOptions,
+    matches: &getopts::Matches,
+    is_unstable_enabled: bool,
+    error_format: ErrorOutputType,
+) -> Vec<PrintRequest> {
     let mut prints = Vec::<PrintRequest>::new();
     if cg.target_cpu.as_ref().map_or(false, |s| s == "help") {
         prints.push(PrintRequest::TargetCPUs);
@@ -2230,72 +2236,105 @@ pub fn build_session_options_and_crate_config(
         prints.push(PrintRequest::CodeModels);
         cg.code_model = None;
     }
-    if debugging_opts
+    if dopts
         .tls_model
         .as_ref()
         .map_or(false, |s| s == "help")
     {
         prints.push(PrintRequest::TlsModels);
-        debugging_opts.tls_model = None;
+        dopts.tls_model = None;
     }
 
-    let cg = cg;
+    prints.extend(matches.opt_strs("print").into_iter().map(|s| match &*s {
+        "crate-name" => PrintRequest::CrateName,
+        "file-names" => PrintRequest::FileNames,
+        "sysroot" => PrintRequest::Sysroot,
+        "cfg" => PrintRequest::Cfg,
+        "target-list" => PrintRequest::TargetList,
+        "target-cpus" => PrintRequest::TargetCPUs,
+        "target-features" => PrintRequest::TargetFeatures,
+        "relocation-models" => PrintRequest::RelocationModels,
+        "code-models" => PrintRequest::CodeModels,
+        "tls-models" => PrintRequest::TlsModels,
+        "native-static-libs" => PrintRequest::NativeStaticLibs,
+        "target-spec-json" => {
+            if is_unstable_enabled {
+                PrintRequest::TargetSpec
+            } else {
+                early_error(
+                    error_format,
+                    "the `-Z unstable-options` flag must also be passed to \
+                     enable the target-spec-json print option",
+                );
+            }
+        }
+        req => early_error(error_format, &format!("unknown print request `{}`", req)),
+    }));
 
-    let sysroot_opt = matches.opt_str("sysroot").map(|m| PathBuf::from(&m));
-    let target_triple = if let Some(target) = matches.opt_str("target") {
-        if target.ends_with(".json") {
+    prints
+}
+
+fn parse_target_triple(matches: &getopts::Matches, error_format: ErrorOutputType) -> TargetTriple {
+    match matches.opt_str("target") {
+        Some(target) if target.ends_with(".json") => {
             let path = Path::new(&target);
             TargetTriple::from_path(&path).unwrap_or_else(|_|
                 early_error(error_format, &format!("target file {:?} does not exist", path)))
+        }
+        Some(target) => TargetTriple::TargetTriple(target),
+        _ => TargetTriple::from_triple(host_triple()),
+    }
+}
+
+fn parse_opt_level(
+    matches: &getopts::Matches,
+    cg: &CodegenOptions,
+    error_format: ErrorOutputType,
+) -> OptLevel {
+    // The `-O` and `-C opt-level` flags specify the same setting, so we want to be able
+    // to use them interchangeably. However, because they're technically different flags,
+    // we need to work out manually which should take precedence if both are supplied (i.e.
+    // the rightmost flag). We do this by finding the (rightmost) position of both flags and
+    // comparing them. Note that if a flag is not found, its position will be `None`, which
+    // always compared less than `Some(_)`.
+    let max_o = matches.opt_positions("O").into_iter().max();
+    let max_c = matches.opt_strs_pos("C").into_iter().flat_map(|(i, s)| {
+        if let Some("opt-level") = s.splitn(2, '=').next() {
+            Some(i)
         } else {
-            TargetTriple::TargetTriple(target)
+            None
         }
+    }).max();
+    if max_o > max_c {
+        OptLevel::Default
     } else {
-        TargetTriple::from_triple(host_triple())
-    };
-    let opt_level = {
-        // The `-O` and `-C opt-level` flags specify the same setting, so we want to be able
-        // to use them interchangeably. However, because they're technically different flags,
-        // we need to work out manually which should take precedence if both are supplied (i.e.
-        // the rightmost flag). We do this by finding the (rightmost) position of both flags and
-        // comparing them. Note that if a flag is not found, its position will be `None`, which
-        // always compared less than `Some(_)`.
-        let max_o = matches.opt_positions("O").into_iter().max();
-        let max_c = matches.opt_strs_pos("C").into_iter().flat_map(|(i, s)| {
-            if let Some("opt-level") = s.splitn(2, '=').next() {
-                Some(i)
-            } else {
-                None
-            }
-        }).max();
-        if max_o > max_c {
-            OptLevel::Default
-        } else {
-            match cg.opt_level.as_ref().map(String::as_ref) {
-                None => OptLevel::No,
-                Some("0") => OptLevel::No,
-                Some("1") => OptLevel::Less,
-                Some("2") => OptLevel::Default,
-                Some("3") => OptLevel::Aggressive,
-                Some("s") => OptLevel::Size,
-                Some("z") => OptLevel::SizeMin,
-                Some(arg) => {
-                    early_error(
-                        error_format,
-                        &format!(
-                            "optimization level needs to be \
-                             between 0-3, s or z (instead was `{}`)",
-                            arg
-                        ),
-                    );
-                }
+        match cg.opt_level.as_ref().map(String::as_ref) {
+            None => OptLevel::No,
+            Some("0") => OptLevel::No,
+            Some("1") => OptLevel::Less,
+            Some("2") => OptLevel::Default,
+            Some("3") => OptLevel::Aggressive,
+            Some("s") => OptLevel::Size,
+            Some("z") => OptLevel::SizeMin,
+            Some(arg) => {
+                early_error(
+                    error_format,
+                    &format!(
+                        "optimization level needs to be \
+                            between 0-3, s or z (instead was `{}`)",
+                        arg
+                    ),
+                );
             }
         }
-    };
-    // The `-g` and `-C debuginfo` flags specify the same setting, so we want to be able
-    // to use them interchangeably. See the note above (regarding `-O` and `-C opt-level`)
-    // for more details.
-    let debug_assertions = cg.debug_assertions.unwrap_or(opt_level == OptLevel::No);
+    }
+}
+
+fn select_debuginfo(
+    matches: &getopts::Matches,
+    cg: &CodegenOptions,
+    error_format: ErrorOutputType,
+) -> DebugInfo {
     let max_g = matches.opt_positions("g").into_iter().max();
     let max_c = matches.opt_strs_pos("C").into_iter().flat_map(|(i, s)| {
         if let Some("debuginfo") = s.splitn(2, '=').next() {
@@ -2304,7 +2343,7 @@ pub fn build_session_options_and_crate_config(
             None
         }
     }).max();
-    let debuginfo = if max_g > max_c {
+    if max_g > max_c {
         DebugInfo::Full
     } else {
         match cg.debuginfo {
@@ -2322,14 +2361,14 @@ pub fn build_session_options_and_crate_config(
                 );
             }
         }
-    };
-
-    let mut search_paths = vec![];
-    for s in &matches.opt_strs("L") {
-        search_paths.push(SearchPath::from_cli_opt(&s[..], error_format));
     }
+}
 
-    let libs = matches
+fn parse_libs(
+    matches: &getopts::Matches,
+    error_format: ErrorOutputType,
+) -> Vec<(String, Option<String>, Option<cstore::NativeLibraryKind>)> {
+    matches
         .opt_strs("l")
         .into_iter()
         .map(|s| {
@@ -2368,52 +2407,23 @@ pub fn build_session_options_and_crate_config(
             let new_name = name_parts.next();
             (name.to_owned(), new_name.map(|n| n.to_owned()), kind)
         })
-        .collect();
-
-    let cfg = parse_cfgspecs(matches.opt_strs("cfg"));
-    let test = matches.opt_present("test");
-
-    let is_unstable_enabled = nightly_options::is_unstable_enabled(matches);
-
-    prints.extend(matches.opt_strs("print").into_iter().map(|s| match &*s {
-        "crate-name" => PrintRequest::CrateName,
-        "file-names" => PrintRequest::FileNames,
-        "sysroot" => PrintRequest::Sysroot,
-        "cfg" => PrintRequest::Cfg,
-        "target-list" => PrintRequest::TargetList,
-        "target-cpus" => PrintRequest::TargetCPUs,
-        "target-features" => PrintRequest::TargetFeatures,
-        "relocation-models" => PrintRequest::RelocationModels,
-        "code-models" => PrintRequest::CodeModels,
-        "tls-models" => PrintRequest::TlsModels,
-        "native-static-libs" => PrintRequest::NativeStaticLibs,
-        "target-spec-json" => {
-            if is_unstable_enabled {
-                PrintRequest::TargetSpec
-            } else {
-                early_error(
-                    error_format,
-                    "the `-Z unstable-options` flag must also be passed to \
-                     enable the target-spec-json print option",
-                );
-            }
-        }
-        req => early_error(error_format, &format!("unknown print request `{}`", req)),
-    }));
+        .collect()
+}
 
-    let borrowck_mode = match debugging_opts.borrowck.as_ref().map(|s| &s[..]) {
+fn parse_borrowck_mode(dopts: &DebuggingOptions, error_format: ErrorOutputType) -> BorrowckMode {
+    match dopts.borrowck.as_ref().map(|s| &s[..]) {
         None | Some("migrate") => BorrowckMode::Migrate,
         Some("mir") => BorrowckMode::Mir,
         Some(m) => early_error(error_format, &format!("unknown borrowck mode `{}`", m)),
-    };
-
-    if !cg.remark.is_empty() && debuginfo == DebugInfo::None {
-        early_warn(
-            error_format,
-            "-C remark requires \"-C debuginfo=n\" to show source locations",
-        );
     }
+}
 
+fn parse_externs(
+    matches: &getopts::Matches,
+    debugging_opts: &DebuggingOptions,
+    error_format: ErrorOutputType,
+    is_unstable_enabled: bool,
+) -> Externs {
     if matches.opt_present("extern-private") && !debugging_opts.unstable_options {
         early_error(
             ErrorOutputType::default(),
@@ -2454,10 +2464,14 @@ pub fn build_session_options_and_crate_config(
         // flag
         entry.is_private_dep |= private;
     }
+    Externs(externs)
+}
 
-    let crate_name = matches.opt_str("crate-name");
-
-    let remap_path_prefix = matches
+fn parse_remap_path_prefix(
+    matches: &getopts::Matches,
+    error_format: ErrorOutputType
+) -> Vec<(PathBuf, PathBuf)> {
+    matches
         .opt_strs("remap-path-prefix")
         .into_iter()
         .map(|remap| {
@@ -2472,42 +2486,130 @@ pub fn build_session_options_and_crate_config(
                 ),
             }
         })
-        .collect();
+        .collect()
+}
 
-    (
-        Options {
-            crate_types,
-            optimize: opt_level,
-            debuginfo,
-            lint_opts,
-            lint_cap,
-            describe_lints,
-            output_types: OutputTypes(output_types),
-            search_paths,
-            maybe_sysroot: sysroot_opt,
-            target_triple,
-            test,
-            incremental,
-            debugging_opts,
-            prints,
-            borrowck_mode,
-            cg,
+pub fn build_session_options(matches: &getopts::Matches) -> Options {
+    let color = parse_color(matches);
+
+    let edition = parse_crate_edition(matches);
+
+    let (json_rendered, json_artifact_notifications) = parse_json(matches);
+
+    let error_format = parse_error_format(matches, color, json_rendered);
+
+    let unparsed_crate_types = matches.opt_strs("crate-type");
+    let crate_types = parse_crate_types_from_list(unparsed_crate_types)
+        .unwrap_or_else(|e| early_error(error_format, &e[..]));
+
+    let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format);
+
+    let mut debugging_opts = build_debugging_options(matches, error_format);
+    check_debug_option_stability(&debugging_opts, error_format, json_rendered);
+
+    let output_types = parse_output_types(&debugging_opts, matches, error_format);
+
+    let mut cg = build_codegen_options(matches, error_format);
+    let (disable_thinlto, codegen_units) = should_override_cgus_and_disable_thinlto(
+        &output_types,
+        matches,
+        error_format,
+        cg.codegen_units,
+    );
+
+    check_thread_count(&debugging_opts, error_format);
+
+    let incremental = select_incremental_path(&debugging_opts, &cg, error_format);
+
+    if debugging_opts.profile && incremental.is_some() {
+        early_error(
             error_format,
-            externs: Externs(externs),
-            crate_name,
-            alt_std_name: None,
-            libs,
-            unstable_features: UnstableFeatures::from_environment(),
-            debug_assertions,
-            actually_rustdoc: false,
-            cli_forced_codegen_units: codegen_units,
-            cli_forced_thinlto_off: disable_thinlto,
-            remap_path_prefix,
-            edition,
-            json_artifact_notifications,
-        },
-        cfg,
-    )
+            "can't instrument with gcov profiling when compiling incrementally",
+        );
+    }
+
+    if cg.profile_generate.enabled() && cg.profile_use.is_some() {
+        early_error(
+            error_format,
+            "options `-C profile-generate` and `-C profile-use` are exclusive",
+        );
+    }
+
+    let is_unstable_enabled = nightly_options::is_unstable_enabled(matches);
+    let prints = collect_print_requests(
+        &mut cg,
+        &mut debugging_opts,
+        matches,
+        is_unstable_enabled,
+        error_format,
+    );
+
+    let cg = cg;
+
+    let sysroot_opt = matches.opt_str("sysroot").map(|m| PathBuf::from(&m));
+    let target_triple = parse_target_triple(matches, error_format);
+    let opt_level = parse_opt_level(matches, &cg, error_format);
+    // The `-g` and `-C debuginfo` flags specify the same setting, so we want to be able
+    // to use them interchangeably. See the note above (regarding `-O` and `-C opt-level`)
+    // for more details.
+    let debug_assertions = cg.debug_assertions.unwrap_or(opt_level == OptLevel::No);
+    let debuginfo = select_debuginfo(matches, &cg, error_format);
+
+    let mut search_paths = vec![];
+    for s in &matches.opt_strs("L") {
+        search_paths.push(SearchPath::from_cli_opt(&s[..], error_format));
+    }
+
+    let libs = parse_libs(matches, error_format);
+
+    let test = matches.opt_present("test");
+
+    let borrowck_mode = parse_borrowck_mode(&debugging_opts, error_format);
+
+    if !cg.remark.is_empty() && debuginfo == DebugInfo::None {
+        early_warn(
+            error_format,
+            "-C remark requires \"-C debuginfo=n\" to show source locations",
+        );
+    }
+
+    let externs = parse_externs(matches, &debugging_opts, error_format, is_unstable_enabled);
+
+    let crate_name = matches.opt_str("crate-name");
+
+    let remap_path_prefix = parse_remap_path_prefix(matches, error_format);
+
+    Options {
+        crate_types,
+        optimize: opt_level,
+        debuginfo,
+        lint_opts,
+        lint_cap,
+        describe_lints,
+        output_types,
+        search_paths,
+        maybe_sysroot: sysroot_opt,
+        target_triple,
+        test,
+        incremental,
+        debugging_opts,
+        prints,
+        borrowck_mode,
+        cg,
+        error_format,
+        externs,
+        crate_name,
+        alt_std_name: None,
+        libs,
+        unstable_features: UnstableFeatures::from_environment(),
+        debug_assertions,
+        actually_rustdoc: false,
+        cli_forced_codegen_units: codegen_units,
+        cli_forced_thinlto_off: disable_thinlto,
+        remap_path_prefix,
+        edition,
+        json_artifact_notifications,
+    }
 }
 
 pub fn make_crate_type_option() -> RustcOptGroup {
diff --git a/src/librustc/session/config/tests.rs b/src/librustc/session/config/tests.rs
index c117418f63699..061bbdc307fc4 100644
--- a/src/librustc/session/config/tests.rs
+++ b/src/librustc/session/config/tests.rs
@@ -3,8 +3,9 @@ use crate::lint;
 use crate::middle::cstore;
 use crate::session::config::{
     build_configuration,
-    build_session_options_and_crate_config,
-    to_crate_config
+    build_session_options,
+    to_crate_config,
+    parse_cfgspecs,
 };
 use crate::session::config::{LtoCli, LinkerPluginLto, SwitchWithOptPath, ExternEntry};
 use crate::session::build_session;
@@ -18,6 +19,16 @@ use syntax::symbol::sym;
 use syntax::edition::{Edition, DEFAULT_EDITION};
 use syntax;
 use super::Options;
+use rustc_data_structures::fx::FxHashSet;
+
+pub fn build_session_options_and_crate_config(
+    matches: &getopts::Matches,
+) -> (Options, FxHashSet<(String, Option<String>)>) {
+    (
+        build_session_options(matches),
+        parse_cfgspecs(matches.opt_strs("cfg")),
+    )
+}
 
 impl ExternEntry {
     fn new_public<S: Into<String>,
diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs
index dd088b68a239a..806afbd1af68c 100644
--- a/src/librustc_driver/lib.rs
+++ b/src/librustc_driver/lib.rs
@@ -166,7 +166,8 @@ pub fn run_compiler(
         None => return Ok(()),
     };
 
-    let (sopts, cfg) = config::build_session_options_and_crate_config(&matches);
+    let sopts = config::build_session_options(&matches);
+    let cfg = config::parse_cfgspecs(matches.opt_strs("cfg"));
 
     let mut dummy_config = |sopts, cfg, diagnostic_output| {
         let mut config = interface::Config {