Rollup merge of rust-lang#76872 - khyperia:remove_declare_methods, r=eddyb

Remove DeclareMethods

Most of the `DeclareMethods` API was only used internally by rustc_codegen_llvm. As such, it makes no sense to require other backends to implement them.

(`get_declared_value` and `declare_cfn` were used, in one place, specific to the `main` symbol, which I've replaced with a more specialized function to allow more flexibility in implementation - the intent is that `declare_c_main` can go away once we do something more clever, e.g. @eddyb has ideas around having a MIR shim or somesuch we can explore in a follow-up PR)

Author: RalfJung

compiler/rustc_codegen_llvm/src/context.rs

@@ -433,6 +433,17 @@ impl MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> {
             llvm::LLVMSetSection(g, section.as_ptr());
         }
     }
+
+    fn declare_c_main(&self, fn_type: Self::Type) -> Option<Self::Function> {
+        if self.get_declared_value("main").is_none() {
+            Some(self.declare_cfn("main", fn_type))
+        } else {
+            // If the symbol already exists, it is an error: for example, the user wrote
+            // #[no_mangle] extern "C" fn main(..) {..}
+            // instead of #[start]
+            None
+        }
+    }
 }
 
 impl CodegenCx<'b, 'tcx> {

compiler/rustc_codegen_llvm/src/declare.rs

@@ -51,42 +51,69 @@ fn declare_raw_fn(
     llfn
 }
 
-impl DeclareMethods<'tcx> for CodegenCx<'ll, 'tcx> {
-    fn declare_global(&self, name: &str, ty: &'ll Type) -> &'ll Value {
+impl CodegenCx<'ll, 'tcx> {
+    /// Declare a global value.
+    ///
+    /// If there’s a value with the same name already declared, the function will
+    /// return its Value instead.
+    pub fn declare_global(&self, name: &str, ty: &'ll Type) -> &'ll Value {
         debug!("declare_global(name={:?})", name);
         unsafe { llvm::LLVMRustGetOrInsertGlobal(self.llmod, name.as_ptr().cast(), name.len(), ty) }
     }
 
-    fn declare_cfn(&self, name: &str, fn_type: &'ll Type) -> &'ll Value {
+    /// Declare a C ABI function.
+    ///
+    /// Only use this for foreign function ABIs and glue. For Rust functions use
+    /// `declare_fn` instead.
+    ///
+    /// If there’s a value with the same name already declared, the function will
+    /// update the declaration and return existing Value instead.
+    pub fn declare_cfn(&self, name: &str, fn_type: &'ll Type) -> &'ll Value {
         declare_raw_fn(self, name, llvm::CCallConv, fn_type)
     }
 
-    fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> &'ll Value {
+    /// Declare a Rust function.
+    ///
+    /// If there’s a value with the same name already declared, the function will
+    /// update the declaration and return existing Value instead.
+    pub fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> &'ll Value {
         debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi);
 
         let llfn = declare_raw_fn(self, name, fn_abi.llvm_cconv(), fn_abi.llvm_type(self));
         fn_abi.apply_attrs_llfn(self, llfn);
         llfn
     }
 
-    fn define_global(&self, name: &str, ty: &'ll Type) -> Option<&'ll Value> {
+    /// Declare a global with an intention to define it.
+    ///
+    /// Use this function when you intend to define a global. This function will
+    /// return `None` if the name already has a definition associated with it. In that
+    /// case an error should be reported to the user, because it usually happens due
+    /// to user’s fault (e.g., misuse of `#[no_mangle]` or `#[export_name]` attributes).
+    pub fn define_global(&self, name: &str, ty: &'ll Type) -> Option<&'ll Value> {
         if self.get_defined_value(name).is_some() {
             None
         } else {
             Some(self.declare_global(name, ty))
         }
     }
 
-    fn define_private_global(&self, ty: &'ll Type) -> &'ll Value {
+    /// Declare a private global
+    ///
+    /// Use this function when you intend to define a global without a name.
+    pub fn define_private_global(&self, ty: &'ll Type) -> &'ll Value {
         unsafe { llvm::LLVMRustInsertPrivateGlobal(self.llmod, ty) }
     }
 
-    fn get_declared_value(&self, name: &str) -> Option<&'ll Value> {
+    /// Gets declared value by name.
+    pub fn get_declared_value(&self, name: &str) -> Option<&'ll Value> {
         debug!("get_declared_value(name={:?})", name);
         unsafe { llvm::LLVMRustGetNamedValue(self.llmod, name.as_ptr().cast(), name.len()) }
     }
 
-    fn get_defined_value(&self, name: &str) -> Option<&'ll Value> {
+    /// Gets defined or externally defined (AvailableExternally linkage) value by
+    /// name.
+    pub fn get_defined_value(&self, name: &str) -> Option<&'ll Value> {
         self.get_declared_value(name).and_then(|val| {
             let declaration = unsafe { llvm::LLVMIsDeclaration(val) != 0 };
             if !declaration { Some(val) } else { None }

compiler/rustc_codegen_ssa/src/base.rs

@@ -407,16 +407,18 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         // listing.
         let main_ret_ty = cx.tcx().erase_regions(&main_ret_ty.no_bound_vars().unwrap());
 
-        if cx.get_declared_value("main").is_some() {
-            // FIXME: We should be smart and show a better diagnostic here.
-            cx.sess()
-                .struct_span_err(sp, "entry symbol `main` declared multiple times")
-                .help("did you use `#[no_mangle]` on `fn main`? Use `#[start]` instead")
-                .emit();
-            cx.sess().abort_if_errors();
-            bug!();
-        }
-        let llfn = cx.declare_cfn("main", llfty);
+        let llfn = match cx.declare_c_main(llfty) {
+            Some(llfn) => llfn,
+            None => {
+                // FIXME: We should be smart and show a better diagnostic here.
+                cx.sess()
+                    .struct_span_err(sp, "entry symbol `main` declared multiple times")
+                    .help("did you use `#[no_mangle]` on `fn main`? Use `#[start]` instead")
+                    .emit();
+                cx.sess().abort_if_errors();
+                bug!();
+            }
+        };
 
         // `main` should respect same config for frame pointer elimination as rest of code
         cx.set_frame_pointer_elimination(llfn);

compiler/rustc_codegen_ssa/src/traits/declare.rs

@@ -1,51 +1,7 @@
 use super::BackendTypes;
 use rustc_hir::def_id::DefId;
 use rustc_middle::mir::mono::{Linkage, Visibility};
-use rustc_middle::ty::{Instance, Ty};
-use rustc_target::abi::call::FnAbi;
-
-pub trait DeclareMethods<'tcx>: BackendTypes {
-    /// Declare a global value.
-    ///
-    /// If there’s a value with the same name already declared, the function will
-    /// return its Value instead.
-    fn declare_global(&self, name: &str, ty: Self::Type) -> Self::Value;
-
-    /// Declare a C ABI function.
-    ///
-    /// Only use this for foreign function ABIs and glue. For Rust functions use
-    /// `declare_fn` instead.
-    ///
-    /// If there’s a value with the same name already declared, the function will
-    /// update the declaration and return existing Value instead.
-    fn declare_cfn(&self, name: &str, fn_type: Self::Type) -> Self::Function;
-
-    /// Declare a Rust function.
-    ///
-    /// If there’s a value with the same name already declared, the function will
-    /// update the declaration and return existing Value instead.
-    fn declare_fn(&self, name: &str, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> Self::Function;
-
-    /// Declare a global with an intention to define it.
-    ///
-    /// Use this function when you intend to define a global. This function will
-    /// return `None` if the name already has a definition associated with it. In that
-    /// case an error should be reported to the user, because it usually happens due
-    /// to user’s fault (e.g., misuse of `#[no_mangle]` or `#[export_name]` attributes).
-    fn define_global(&self, name: &str, ty: Self::Type) -> Option<Self::Value>;
-
-    /// Declare a private global
-    ///
-    /// Use this function when you intend to define a global without a name.
-    fn define_private_global(&self, ty: Self::Type) -> Self::Value;
-
-    /// Gets declared value by name.
-    fn get_declared_value(&self, name: &str) -> Option<Self::Value>;
-
-    /// Gets defined or externally defined (AvailableExternally linkage) value by
-    /// name.
-    fn get_defined_value(&self, name: &str) -> Option<Self::Value>;
-}
+use rustc_middle::ty::Instance;
 
 pub trait PreDefineMethods<'tcx>: BackendTypes {
     fn predefine_static(

compiler/rustc_codegen_ssa/src/traits/misc.rs

@@ -19,4 +19,6 @@ pub trait MiscMethods<'tcx>: BackendTypes {
     fn set_frame_pointer_elimination(&self, llfn: Self::Function);
     fn apply_target_cpu_attr(&self, llfn: Self::Function);
     fn create_used_variable(&self);
+    /// Declares the extern "C" main function for the entry point. Returns None if the symbol already exists.
+    fn declare_c_main(&self, fn_type: Self::Type) -> Option<Self::Function>;
 }

compiler/rustc_codegen_ssa/src/traits/mod.rs

@@ -35,7 +35,7 @@ pub use self::builder::{BuilderMethods, OverflowOp};
 pub use self::consts::ConstMethods;
 pub use self::coverageinfo::{CoverageInfoBuilderMethods, CoverageInfoMethods};
 pub use self::debuginfo::{DebugInfoBuilderMethods, DebugInfoMethods};
-pub use self::declare::{DeclareMethods, PreDefineMethods};
+pub use self::declare::PreDefineMethods;
 pub use self::intrinsic::IntrinsicCallMethods;
 pub use self::misc::MiscMethods;
 pub use self::statics::{StaticBuilderMethods, StaticMethods};
@@ -60,7 +60,6 @@ pub trait CodegenMethods<'tcx>:
     + StaticMethods
     + CoverageInfoMethods
     + DebugInfoMethods<'tcx>
-    + DeclareMethods<'tcx>
     + AsmMethods
     + PreDefineMethods<'tcx>
     + HasParamEnv<'tcx>
@@ -77,7 +76,6 @@ impl<'tcx, T> CodegenMethods<'tcx> for T where
         + StaticMethods
         + CoverageInfoMethods
         + DebugInfoMethods<'tcx>
-        + DeclareMethods<'tcx>
         + AsmMethods
         + PreDefineMethods<'tcx>
         + HasParamEnv<'tcx>