Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc_session: Cleanup session creation #72669

Merged
merged 1 commit into from
May 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/librustc_interface/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_session::lint;
use rustc_session::parse::{CrateConfig, ParseSess};
use rustc_session::{DiagnosticOutput, Session};
use rustc_span::edition;
use rustc_span::source_map::{FileLoader, FileName, SourceMap};
use rustc_span::source_map::{FileLoader, FileName};
use std::path::PathBuf;
use std::result;
use std::sync::{Arc, Mutex};
Expand All @@ -31,7 +31,6 @@ pub type Result<T> = result::Result<T, ErrorReported>;
pub struct Compiler {
pub(crate) sess: Lrc<Session>,
codegen_backend: Lrc<Box<dyn CodegenBackend>>,
source_map: Lrc<SourceMap>,
pub(crate) input: Input,
pub(crate) input_path: Option<PathBuf>,
pub(crate) output_dir: Option<PathBuf>,
Expand All @@ -49,9 +48,6 @@ impl Compiler {
pub fn codegen_backend(&self) -> &Lrc<Box<dyn CodegenBackend>> {
&self.codegen_backend
}
pub fn source_map(&self) -> &Lrc<SourceMap> {
&self.source_map
}
pub fn input(&self) -> &Input {
&self.input
}
Expand Down Expand Up @@ -168,7 +164,7 @@ pub fn run_compiler_in_existing_thread_pool<R>(
f: impl FnOnce(&Compiler) -> R,
) -> R {
let registry = &config.registry;
let (sess, codegen_backend, source_map) = util::create_session(
let (sess, codegen_backend) = util::create_session(
config.opts,
config.crate_cfg,
config.diagnostic_output,
Expand All @@ -181,7 +177,6 @@ pub fn run_compiler_in_existing_thread_pool<R>(
let compiler = Compiler {
sess,
codegen_backend,
source_map,
input: config.input,
input_path: config.input_path,
output_dir: config.output_dir,
Expand Down
12 changes: 9 additions & 3 deletions src/librustc_interface/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ use rustc_session::config::{build_configuration, build_session_options, to_crate
use rustc_session::config::{rustc_optgroups, ErrorOutputType, ExternLocation, Options, Passes};
use rustc_session::config::{CFGuard, ExternEntry, LinkerPluginLto, LtoCli, SwitchWithOptPath};
use rustc_session::config::{Externs, OutputType, OutputTypes, Sanitizer, SymbolManglingVersion};
use rustc_session::getopts;
use rustc_session::lint::Level;
use rustc_session::search_paths::SearchPath;
use rustc_session::utils::NativeLibKind;
use rustc_session::{build_session, Session};
use rustc_session::{build_session, getopts, DiagnosticOutput, Session};
use rustc_span::edition::{Edition, DEFAULT_EDITION};
use rustc_span::symbol::sym;
use rustc_span::SourceFileHashAlgorithm;
Expand All @@ -32,7 +31,14 @@ fn build_session_options_and_crate_config(matches: getopts::Matches) -> (Options
fn mk_session(matches: getopts::Matches) -> (Session, CfgSpecs) {
let registry = registry::Registry::new(&[]);
let (sessopts, cfg) = build_session_options_and_crate_config(matches);
let sess = build_session(sessopts, None, registry);
let sess = build_session(
sessopts,
None,
registry,
DiagnosticOutput::Default,
Default::default(),
None,
);
(sess, cfg)
}

Expand Down
8 changes: 4 additions & 4 deletions src/librustc_interface/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rustc_session::parse::CrateConfig;
use rustc_session::CrateDisambiguator;
use rustc_session::{early_error, filesearch, output, DiagnosticOutput, Session};
use rustc_span::edition::Edition;
use rustc_span::source_map::{FileLoader, SourceMap};
use rustc_span::source_map::FileLoader;
use rustc_span::symbol::{sym, Symbol};
use smallvec::SmallVec;
use std::env;
Expand Down Expand Up @@ -65,8 +65,8 @@ pub fn create_session(
input_path: Option<PathBuf>,
lint_caps: FxHashMap<lint::LintId, lint::Level>,
descriptions: Registry,
) -> (Lrc<Session>, Lrc<Box<dyn CodegenBackend>>, Lrc<SourceMap>) {
let (mut sess, source_map) = session::build_session_with_source_map(
) -> (Lrc<Session>, Lrc<Box<dyn CodegenBackend>>) {
let mut sess = session::build_session(
sopts,
input_path,
descriptions,
Expand All @@ -81,7 +81,7 @@ pub fn create_session(
add_configuration(&mut cfg, &mut sess, &*codegen_backend);
sess.parse_sess.config = cfg;

(Lrc::new(sess), Lrc::new(codegen_backend), source_map)
(Lrc::new(sess), Lrc::new(codegen_backend))
}

const STACK_SIZE: usize = 8 * 1024 * 1024;
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_session/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ impl ParseSess {
&self.source_map
}

pub fn clone_source_map(&self) -> Lrc<SourceMap> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, should we avoid the clone_ here since it's really cheap to clone it (since it's just an Arc)?

Alternatively maybe the previously existing method can be made to return &Lrc<SourceMap> and then we could clone that in callers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only used once from rustdoc, source map doesn't generally need cloning.
Perhaps we'll be able to replace Lrc<SourceMap> with just SourceMap in the future.
So I'd rather keep this method explicit / visible for now.

self.source_map.clone()
}

pub fn buffer_lint(
&self,
lint: &'static Lint,
Expand Down
55 changes: 15 additions & 40 deletions src/librustc_session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_errors::json::JsonEmitter;
use rustc_errors::registry::Registry;
use rustc_errors::{Applicability, DiagnosticBuilder, DiagnosticId, ErrorReported};
use rustc_span::edition::Edition;
use rustc_span::source_map::{self, FileLoader, MultiSpan, RealFileLoader, SourceMap, Span};
use rustc_span::source_map::{FileLoader, MultiSpan, RealFileLoader, SourceMap, Span};
use rustc_span::{SourceFileHashAlgorithm, Symbol};
use rustc_target::asm::InlineAsmArch;
use rustc_target::spec::{CodeModel, PanicStrategy, RelocModel, RelroLevel};
Expand Down Expand Up @@ -481,7 +481,7 @@ impl Session {
}

#[inline]
pub fn source_map(&self) -> &source_map::SourceMap {
pub fn source_map(&self) -> &SourceMap {
self.parse_sess.source_map()
}
pub fn verbose(&self) -> bool {
Expand Down Expand Up @@ -984,26 +984,10 @@ impl Session {
}
}

pub fn build_session(
sopts: config::Options,
local_crate_source_file: Option<PathBuf>,
registry: rustc_errors::registry::Registry,
) -> Session {
build_session_with_source_map(
sopts,
local_crate_source_file,
registry,
DiagnosticOutput::Default,
Default::default(),
None,
)
.0
}

fn default_emitter(
sopts: &config::Options,
registry: rustc_errors::registry::Registry,
source_map: &Lrc<source_map::SourceMap>,
source_map: Lrc<SourceMap>,
emitter_dest: Option<Box<dyn Write + Send>>,
) -> Box<dyn Emitter + sync::Send> {
let macro_backtrace = sopts.debugging_opts.macro_backtrace;
Expand All @@ -1012,25 +996,22 @@ fn default_emitter(
let (short, color_config) = kind.unzip();

if let HumanReadableErrorType::AnnotateSnippet(_) = kind {
let emitter = AnnotateSnippetEmitterWriter::new(
Some(source_map.clone()),
short,
macro_backtrace,
);
let emitter =
AnnotateSnippetEmitterWriter::new(Some(source_map), short, macro_backtrace);
Box::new(emitter.ui_testing(sopts.debugging_opts.ui_testing))
} else {
let emitter = match dst {
None => EmitterWriter::stderr(
color_config,
Some(source_map.clone()),
Some(source_map),
short,
sopts.debugging_opts.teach,
sopts.debugging_opts.terminal_width,
macro_backtrace,
),
Some(dst) => EmitterWriter::new(
dst,
Some(source_map.clone()),
Some(source_map),
short,
false, // no teach messages when writing to a buffer
false, // no colors when writing to a buffer
Expand All @@ -1042,20 +1023,14 @@ fn default_emitter(
}
}
(config::ErrorOutputType::Json { pretty, json_rendered }, None) => Box::new(
JsonEmitter::stderr(
Some(registry),
source_map.clone(),
pretty,
json_rendered,
macro_backtrace,
)
.ui_testing(sopts.debugging_opts.ui_testing),
JsonEmitter::stderr(Some(registry), source_map, pretty, json_rendered, macro_backtrace)
.ui_testing(sopts.debugging_opts.ui_testing),
),
(config::ErrorOutputType::Json { pretty, json_rendered }, Some(dst)) => Box::new(
JsonEmitter::new(
dst,
Some(registry),
source_map.clone(),
source_map,
pretty,
json_rendered,
macro_backtrace,
Expand All @@ -1070,14 +1045,14 @@ pub enum DiagnosticOutput {
Raw(Box<dyn Write + Send>),
}

pub fn build_session_with_source_map(
pub fn build_session(
sopts: config::Options,
local_crate_source_file: Option<PathBuf>,
registry: rustc_errors::registry::Registry,
diagnostics_output: DiagnosticOutput,
driver_lint_caps: FxHashMap<lint::LintId, lint::Level>,
file_loader: Option<Box<dyn FileLoader + Send + Sync + 'static>>,
) -> (Session, Lrc<SourceMap>) {
) -> Session {
// FIXME: This is not general enough to make the warning lint completely override
// normal diagnostic warnings, since the warning lint can also be denied and changed
// later via the source code.
Expand Down Expand Up @@ -1115,7 +1090,7 @@ pub fn build_session_with_source_map(
sopts.file_path_mapping(),
hash_kind,
));
let emitter = default_emitter(&sopts, registry, &source_map, write_dest);
let emitter = default_emitter(&sopts, registry, source_map.clone(), write_dest);

let span_diagnostic = rustc_errors::Handler::with_emitter_and_flags(
emitter,
Expand Down Expand Up @@ -1143,7 +1118,7 @@ pub fn build_session_with_source_map(
None
};

let parse_sess = ParseSess::with_span_handler(span_diagnostic, source_map.clone());
let parse_sess = ParseSess::with_span_handler(span_diagnostic, source_map);
let sysroot = match &sopts.maybe_sysroot {
Some(sysroot) => sysroot.clone(),
None => filesearch::get_or_default_sysroot(),
Expand Down Expand Up @@ -1266,7 +1241,7 @@ pub fn build_session_with_source_map(

validate_commandline_args_with_session_available(&sess);

(sess, source_map)
sess
}

// If it is useful to have a Session available already for validating a
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub fn run(options: Options) -> Result<(), String> {
options,
false,
opts,
Some(compiler.source_map().clone()),
Some(compiler.session().parse_sess.clone_source_map()),
None,
enable_per_target_ignores,
);
Expand Down