Skip to content

Mostly fix metadata_only backend and extract some code out of rustc_codegen_llvm #51590

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

Merged
merged 19 commits into from
Jul 8, 2018
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
8 changes: 0 additions & 8 deletions src/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ dependencies = [
"winapi 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "ar"
version = "0.3.1"
source = "registry+https://github.com/rust-lang/crates.io-index"

[[package]]
name = "arena"
version = "0.0.0"
Expand Down Expand Up @@ -2031,7 +2026,6 @@ dependencies = [
name = "rustc_codegen_utils"
version = "0.0.0"
dependencies = [
"ar 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
Copy link
Member

Choose a reason for hiding this comment

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

Does something else depend on this crate?

Copy link
Member Author

Choose a reason for hiding this comment

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

No

Copy link
Member

Choose a reason for hiding this comment

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

I was asking because there's a "checksum" line that would also be removed if this was the last use of ar.
I just checked and rustc_driver also depends on ar.

Copy link
Member Author

Choose a reason for hiding this comment

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

what??? how did tidy pass then? i removed ar as whitelisted dep

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed unused dep

"flate2 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc 0.0.0",
Expand Down Expand Up @@ -2072,7 +2066,6 @@ dependencies = [
name = "rustc_driver"
version = "0.0.0"
dependencies = [
"ar 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
"arena 0.0.0",
"env_logger 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)",
"graphviz 0.0.0",
Expand Down Expand Up @@ -3077,7 +3070,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum aho-corasick 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)" = "f0ba20154ea1f47ce2793322f049c5646cc6d0fa9759d5f333f286e507bf8080"
"checksum ammonia 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fd4c682378117e4186a492b2252b9537990e1617f44aed9788b9a1149de45477"
"checksum ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b"
"checksum ar 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "35c7a5669cb64f085739387e1308b74e6d44022464b7f1b63bbd4ceb6379ec31"
"checksum arrayvec 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)" = "a1e964f9e24d588183fcb43503abda40d288c8657dfc27311516ce2f05675aef"
"checksum assert_cli 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "98589b0e465a6c510d95fceebd365bb79bedece7f6e18a480897f2015f85ec51"
"checksum atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)" = "2fc4a1aa4c24c0718a250f0681885c1af91419d242f29eb8f2ab28502d80dbd1"
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ pub mod util {
pub mod ppaux;
pub mod nodemap;
pub mod fs;
pub mod time_graph;
}

// A private module so that macro-expanded idents like
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_codegen_llvm/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ use builder::{Builder, MemFlags};
use callee;
use common::{C_bool, C_bytes_in_context, C_i32, C_usize};
use rustc_mir::monomorphize::collector::{self, MonoItemCollectionMode};
use rustc_mir::monomorphize::item::DefPathBasedNames;
use common::{self, C_struct_in_context, C_array, val_ty};
use consts;
use context::{self, CodegenCx};
Expand All @@ -67,7 +68,7 @@ use monomorphize::Instance;
use monomorphize::partitioning::{self, PartitioningStrategy, CodegenUnit, CodegenUnitExt};
use rustc_codegen_utils::symbol_names_test;
use time_graph;
use mono_item::{MonoItem, BaseMonoItemExt, MonoItemExt, DefPathBasedNames};
use mono_item::{MonoItem, BaseMonoItemExt, MonoItemExt};
use type_::Type;
use type_of::LayoutLlvmExt;
use rustc::util::nodemap::{FxHashMap, FxHashSet, DefIdSet};
Expand All @@ -92,7 +93,7 @@ use syntax::ast;

use mir::operand::OperandValue;

pub use rustc_codegen_utils::check_for_rustc_errors_attr;
use rustc_codegen_utils::check_for_rustc_errors_attr;

pub struct StatRecorder<'a, 'tcx: 'a> {
cx: &'a CodegenCx<'a, 'tcx>,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_llvm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ use rustc::middle::lang_items::LangItem;
use rustc::session::{Session, CompileIncomplete};
use rustc::session::config::{OutputFilenames, OutputType, PrintRequest};
use rustc::ty::{self, TyCtxt};
use rustc::util::time_graph;
use rustc::util::nodemap::{FxHashSet, FxHashMap};
use rustc_mir::monomorphize;
use rustc_codegen_utils::codegen_backend::CodegenBackend;
Expand Down Expand Up @@ -114,7 +115,6 @@ mod llvm_util;
mod metadata;
mod meth;
mod mir;
mod time_graph;
mod mono_item;
mod type_;
mod type_of;
Expand Down Expand Up @@ -368,7 +368,7 @@ struct CodegenResults {
crate_info: CrateInfo,
}

// Misc info we load from metadata to persist beyond the tcx
/// Misc info we load from metadata to persist beyond the tcx
struct CrateInfo {
panic_runtime: Option<CrateNum>,
compiler_builtins: Option<CrateNum>,
Expand Down
1 change: 0 additions & 1 deletion src/librustc_codegen_llvm/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ use std::fmt;

pub use rustc::mir::mono::MonoItem;

pub use rustc_mir::monomorphize::item::*;
pub use rustc_mir::monomorphize::item::MonoItemExt as BaseMonoItemExt;

pub trait MonoItemExt<'a, 'tcx>: fmt::Debug + BaseMonoItemExt<'a, 'tcx> {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc::ty::{self, Ty, TypeFoldable};
use rustc::ty::layout::{self, Align, LayoutOf, Size, TyLayout};
use rustc_target::spec::PanicStrategy;
use rustc_target::abi::FloatTy;
use mono_item::DefPathBasedNames;
use rustc_mir::monomorphize::item::DefPathBasedNames;
use type_::Type;

use std::fmt::Write;
Expand Down
1 change: 0 additions & 1 deletion src/librustc_codegen_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ crate-type = ["dylib"]
test = false

[dependencies]
ar = "0.3.0"
flate2 = "1.0"
log = "0.4"

Expand Down
103 changes: 19 additions & 84 deletions src/librustc_codegen_utils/codegen_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,13 @@
#![feature(box_syntax)]

use std::any::Any;
use std::io::prelude::*;
use std::io::{self, Cursor};
use std::io::{self, Write};
use std::fs::File;
use std::path::Path;
use std::sync::mpsc;
use std::sync::{mpsc, Arc};

use rustc_data_structures::owning_ref::OwningRef;
use rustc_data_structures::sync::Lrc;
use ar::{Archive, Builder, Header};
use flate2::Compression;
use flate2::write::DeflateEncoder;
Copy link
Member

Choose a reason for hiding this comment

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

What is flate2 used for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Compressing the metadata, when rustc_metadata expects it.


Expand Down Expand Up @@ -81,89 +79,21 @@ pub trait CodegenBackend {
) -> Result<(), CompileIncomplete>;
}

pub struct DummyCodegenBackend;
Copy link
Member Author

Choose a reason for hiding this comment

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

It isn't used anywere

Copy link
Member

Choose a reason for hiding this comment

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

Not even in librustc_driver/test.rs?

Copy link
Member Author

Choose a reason for hiding this comment

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

No


impl CodegenBackend for DummyCodegenBackend {
fn metadata_loader(&self) -> Box<MetadataLoader + Sync> {
box DummyMetadataLoader(())
}

fn provide(&self, _providers: &mut Providers) {
bug!("DummyCodegenBackend::provide");
}

fn provide_extern(&self, _providers: &mut Providers) {
bug!("DummyCodegenBackend::provide_extern");
}

fn codegen_crate<'a, 'tcx>(
&self,
_tcx: TyCtxt<'a, 'tcx, 'tcx>,
_rx: mpsc::Receiver<Box<Any + Send>>
) -> Box<Any> {
bug!("DummyCodegenBackend::codegen_backend");
}

fn join_codegen_and_link(
&self,
_ongoing_codegen: Box<Any>,
_sess: &Session,
_dep_graph: &DepGraph,
_outputs: &OutputFilenames,
) -> Result<(), CompileIncomplete> {
bug!("DummyCodegenBackend::join_codegen_and_link");
}
}

pub struct DummyMetadataLoader(());

impl MetadataLoader for DummyMetadataLoader {
fn get_rlib_metadata(
&self,
_target: &Target,
_filename: &Path
) -> Result<MetadataRef, String> {
bug!("DummyMetadataLoader::get_rlib_metadata");
}

fn get_dylib_metadata(
&self,
_target: &Target,
_filename: &Path
) -> Result<MetadataRef, String> {
bug!("DummyMetadataLoader::get_dylib_metadata");
}
}

pub struct NoLlvmMetadataLoader;

impl MetadataLoader for NoLlvmMetadataLoader {
fn get_rlib_metadata(&self, _: &Target, filename: &Path) -> Result<MetadataRef, String> {
let file = File::open(filename)
let mut file = File::open(filename)
.map_err(|e| format!("metadata file open err: {:?}", e))?;
let mut archive = Archive::new(file);

while let Some(entry_result) = archive.next_entry() {
let mut entry = entry_result
.map_err(|e| format!("metadata section read err: {:?}", e))?;
if entry.header().identifier() == "rust.metadata.bin" {
let mut buf = Vec::new();
io::copy(&mut entry, &mut buf).unwrap();
let buf: OwningRef<Vec<u8>, [u8]> = OwningRef::new(buf).into();
return Ok(rustc_erase_owner!(buf.map_owner_box()));
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

It couldn't load all metadata produced by rustc_codegen_llvm anyway, because it didn't support dylibs.


Err("Couldn't find metadata section".to_string())
let mut buf = Vec::new();
io::copy(&mut file, &mut buf).unwrap();
let buf: OwningRef<Vec<u8>, [u8]> = OwningRef::new(buf).into();
return Ok(rustc_erase_owner!(buf.map_owner_box()));
}

fn get_dylib_metadata(
&self,
_target: &Target,
_filename: &Path,
) -> Result<MetadataRef, String> {
// FIXME: Support reading dylibs from llvm enabled rustc
self.get_rlib_metadata(_target, _filename)
fn get_dylib_metadata(&self, target: &Target, filename: &Path) -> Result<MetadataRef, String> {
self.get_rlib_metadata(target, filename)
}
}

Expand Down Expand Up @@ -205,8 +135,13 @@ impl CodegenBackend for MetadataOnlyCodegenBackend {
providers.target_features_whitelist = |_tcx, _cnum| {
Lrc::new(FxHashMap()) // Just a dummy
};
providers.is_reachable_non_generic = |_tcx, _defid| true;
providers.exported_symbols = |_tcx, _crate| Arc::new(Vec::new());
providers.wasm_custom_sections = |_tcx, _crate| Lrc::new(Vec::new());
}
fn provide_extern(&self, providers: &mut Providers) {
providers.is_reachable_non_generic = |_tcx, _defid| true;
}
fn provide_extern(&self, _providers: &mut Providers) {}

fn codegen_crate<'a, 'tcx>(
&self,
Expand All @@ -225,7 +160,8 @@ impl CodegenBackend for MetadataOnlyCodegenBackend {
collector::MonoItemCollectionMode::Eager
).0.iter()
);
::rustc::middle::dependency_format::calculate(tcx);
// FIXME: Fix this
// ::rustc::middle::dependency_format::calculate(tcx);
let _ = tcx.link_args(LOCAL_CRATE);
let _ = tcx.native_libraries(LOCAL_CRATE);
for mono_item in
Expand Down Expand Up @@ -280,9 +216,8 @@ impl CodegenBackend for MetadataOnlyCodegenBackend {
} else {
&ongoing_codegen.metadata.raw_data
};
let mut builder = Builder::new(File::create(&output_name).unwrap());
let header = Header::new("rust.metadata.bin".to_string(), metadata.len() as u64);
builder.append(&header, Cursor::new(metadata)).unwrap();
let mut file = File::create(&output_name).unwrap();
file.write_all(metadata).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

So all library types are effectively the same as --emit=metadata?

Copy link
Member Author

@bjorn3 bjorn3 Jun 22, 2018

Choose a reason for hiding this comment

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

For the metadata-only backend yes, except for dylibs, because these must have compressed metadata.

}

sess.abort_if_errors();
Expand Down
3 changes: 0 additions & 3 deletions src/librustc_codegen_utils/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

#![recursion_limit="256"]

extern crate ar;
extern crate flate2;
#[macro_use]
extern crate log;
Expand All @@ -39,8 +38,6 @@ extern crate syntax;
extern crate syntax_pos;
#[macro_use] extern crate rustc_data_structures;

pub extern crate rustc as __rustc;

use rustc::ty::TyCtxt;

pub mod link;
Expand Down
2 changes: 0 additions & 2 deletions src/librustc_driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,3 @@ serialize = { path = "../libserialize" }
syntax = { path = "../libsyntax" }
syntax_ext = { path = "../libsyntax_ext" }
syntax_pos = { path = "../libsyntax_pos" }

ar = "0.3.0"
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ all:
/bin/echo || exit 0 # This test requires /bin/echo to exist
$(RUSTC) the_backend.rs --crate-name the_backend --crate-type dylib \
-o $(TMPDIR)/the_backend.dylib
$(RUSTC) some_crate.rs --crate-name some_crate --crate-type bin -o $(TMPDIR)/some_crate \
$(RUSTC) some_crate.rs --crate-name some_crate --crate-type lib -o $(TMPDIR)/some_crate \
-Z codegen-backend=$(TMPDIR)/the_backend.dylib -Z unstable-options
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we have two tests here, one for lib and one for bin ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Bin doesn't work without adding several lang items. some_crate.rs is #![no_core], because libcore and libstd use ar based rlibs, while this pr replaces them with plain metadata for MetadataOnlyCodegenBackend, because it didn't support dylibs anyway.

grep -x "This has been \"compiled\" successfully." $(TMPDIR)/some_crate
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn main() {
::std::process::exit(1);
}
#![feature(no_core)]
#![no_core]
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl CodegenBackend for TheBackend {
let crate_name = ongoing_codegen.downcast::<Symbol>()
.expect("in join_codegen_and_link: ongoing_codegen is not a Symbol");
for &crate_type in sess.opts.crate_types.iter() {
if crate_type != CrateType::CrateTypeExecutable {
if crate_type != CrateType::CrateTypeRlib {
sess.fatal(&format!("Crate type is {:?}", crate_type));
}
let output_name =
Expand Down
1 change: 0 additions & 1 deletion src/tools/tidy/src/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ static WHITELIST_CRATES: &'static [CrateVersion] = &[
/// Whitelist of crates rustc is allowed to depend on. Avoid adding to the list if possible.
static WHITELIST: &'static [Crate] = &[
Crate("aho-corasick"),
Crate("ar"),
Crate("arrayvec"),
Crate("atty"),
Crate("backtrace"),
Expand Down