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.

Sign up for GitHub

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

Codegen: Query span as late as possible #78280

Merged
merged 1 commit into from
Nov 4, 2020
Merged

Codegen: Query span as late as possible #78280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 14 additions & 14 deletions compiler/rustc_codegen_llvm/src/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use rustc_middle::mir::mono::MonoItem;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_middle::{bug, span_bug};
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_target::abi::{AddressSpace, Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size};
use tracing::debug;

Expand Down Expand Up @@ -110,7 +109,7 @@ fn check_and_apply_linkage(
attrs: &CodegenFnAttrs,
ty: Ty<'tcx>,
sym: &str,
span: Span,
span_def_id: DefId,
) -> &'ll Value {
let llty = cx.layout_of(ty).llvm_type(cx);
if let Some(linkage) = attrs.linkage {
Expand All @@ -125,7 +124,7 @@ fn check_and_apply_linkage(
cx.layout_of(mt.ty).llvm_type(cx)
} else {
cx.sess().span_fatal(
span,
cx.tcx.def_span(span_def_id),
"must have type `*const T` or `*mut T` due to `#[linkage]` attribute",
)
};
Expand All @@ -143,7 +142,10 @@ fn check_and_apply_linkage(
let mut real_name = "_rust_extern_with_linkage_".to_string();
real_name.push_str(&sym);
let g2 = cx.define_global(&real_name, llty).unwrap_or_else(|| {
cx.sess().span_fatal(span, &format!("symbol `{}` is already defined", &sym))
cx.sess().span_fatal(
cx.tcx.def_span(span_def_id),
&format!("symbol `{}` is already defined", &sym),
)
});
llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage);
llvm::LLVMSetInitializer(g2, g1);
Expand Down Expand Up @@ -210,21 +212,21 @@ impl CodegenCx<'ll, 'tcx> {

debug!("get_static: sym={} instance={:?}", sym, instance);

let g = if let Some(def_id) = def_id.as_local() {
let id = self.tcx.hir().local_def_id_to_hir_id(def_id);
let g = if let Some(local_def_id) = def_id.as_local() {
let id = self.tcx.hir().local_def_id_to_hir_id(local_def_id);
let llty = self.layout_of(ty).llvm_type(self);
// FIXME: refactor this to work without accessing the HIR
let (g, attrs) = match self.tcx.hir().get(id) {
Node::Item(&hir::Item { attrs, span, kind: hir::ItemKind::Static(..), .. }) => {
Node::Item(&hir::Item { attrs, kind: hir::ItemKind::Static(..), .. }) => {
if let Some(g) = self.get_declared_value(sym) {
if self.val_ty(g) != self.type_ptr_to(llty) {
span_bug!(span, "Conflicting types for static");
span_bug!(self.tcx.def_span(def_id), "Conflicting types for static");
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason you aren't using the Item span here? That seems easier to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment, it is, yes. Consider this change a step towards #43088 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also I actually dont know, by the way, if these are identical, so it's kind of a "let's see what the tests think" kind of change.

Copy link
Contributor

@lcnr lcnr Oct 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

the tests won't think anything here will they? This is a span_bug so we should never reach this in tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GitHub needs a reaction that indicates how blind I can be :)

}
}

let g = self.declare_global(sym, llty);

if !self.tcx.is_reachable_non_generic(def_id) {
if !self.tcx.is_reachable_non_generic(local_def_id) {
unsafe {
llvm::LLVMRustSetVisibility(g, llvm::Visibility::Hidden);
}
Expand All @@ -235,12 +237,11 @@ impl CodegenCx<'ll, 'tcx> {

Node::ForeignItem(&hir::ForeignItem {
ref attrs,
span,
kind: hir::ForeignItemKind::Static(..),
..
}) => {
let fn_attrs = self.tcx.codegen_fn_attrs(def_id);
(check_and_apply_linkage(&self, &fn_attrs, ty, sym, span), &**attrs)
let fn_attrs = self.tcx.codegen_fn_attrs(local_def_id);
(check_and_apply_linkage(&self, &fn_attrs, ty, sym, def_id), &**attrs)
}

item => bug!("get_static: expected static, found {:?}", item),
Expand All @@ -260,8 +261,7 @@ impl CodegenCx<'ll, 'tcx> {
debug!("get_static: sym={} item_attr={:?}", sym, self.tcx.item_attrs(def_id));

let attrs = self.tcx.codegen_fn_attrs(def_id);
let span = self.tcx.def_span(def_id);
let g = check_and_apply_linkage(&self, &attrs, ty, sym, span);
let g = check_and_apply_linkage(&self, &attrs, ty, sym, def_id);

// Thread-local statics in some other crate need to *always* be linked
// against in a thread-local fashion, so we need to be sure to apply the
Expand Down
13 changes: 4 additions & 9 deletions compiler/rustc_codegen_ssa/src/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ use rustc_session::cgu_reuse_tracker::CguReuse;
use rustc_session::config::{self, EntryFnType};
use rustc_session::utils::NativeLibKind;
use rustc_session::Session;
use rustc_span::Span;
use rustc_symbol_mangling::test as symbol_names_test;
use rustc_target::abi::{Align, LayoutOf, VariantIdx};

Expand Down Expand Up @@ -364,11 +363,7 @@ pub fn codegen_instance<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
cx: &'a Bx::CodegenCx,
) -> Option<Bx::Function> {
let (main_def_id, span) = match cx.tcx().entry_fn(LOCAL_CRATE) {
Some((def_id, _)) => (def_id, cx.tcx().def_span(def_id)),
None => return None,
};

let main_def_id = cx.tcx().entry_fn(LOCAL_CRATE).map(|(def_id, _)| def_id)?;
let instance = Instance::mono(cx.tcx(), main_def_id.to_def_id());

if !cx.codegen_unit().contains_item(&MonoItem::Fn(instance)) {
Expand All @@ -381,12 +376,11 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

return cx.tcx().entry_fn(LOCAL_CRATE).map(|(_, et)| {
let use_start_lang_item = EntryFnType::Start != et;
create_entry_fn::<Bx>(cx, span, main_llfn, main_def_id, use_start_lang_item)
create_entry_fn::<Bx>(cx, main_llfn, main_def_id, use_start_lang_item)
});

fn create_entry_fn<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
cx: &'a Bx::CodegenCx,
sp: Span,
rust_main: Bx::Value,
rust_main_def_id: LocalDefId,
use_start_lang_item: bool,
Expand All @@ -411,8 +405,9 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
Some(llfn) => llfn,
None => {
// FIXME: We should be smart and show a better diagnostic here.
let span = cx.tcx().def_span(rust_main_def_id);
cx.sess()
.struct_span_err(sp, "entry symbol `main` declared multiple times")
.struct_span_err(span, "entry symbol `main` declared multiple times")
.help("did you use `#[no_mangle]` on `fn main`? Use `#[start]` instead")
.emit();
cx.sess().abort_if_errors();
Expand Down