Skip to content

Commit

Permalink
Unrolled build for rust-lang#131558
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#131558 - sassman:feat/warnin-for-no-mangle-together-with-export-name, r=Urgau

Lint on combining `#[no_mangle]` and `#[export_name]`

This is my very first contribution to the compiler, even though I read the [chapter about lints](https://rustc-dev-guide.rust-lang.org/diagnostics.html) I'm not very certain that this ~~new lint is done right as a builtin lint~~ PR is right. I appreciate any guidance on how to improve the code.

- Add test for issue rust-lang#47446
- ~~Implement the new lint `mixed_export_name_and_no_mangle` as a builtin lint (not sure if that is the right way to go)~~ Extend `unused_attributes` lint
- Add suggestion how to fix it

<details>

<summary>Old proposed new lint</summary>

> The `mixed_export_name_and_no_mangle` lint detects usage of both `#[export_name]` and `#[no_mangle]` on the same item which results on `#[no_mangle]` being ignored.
>
> *warn-by-default*
>
> ### Example
>
> ```rust
> #[no_mangle] // ignored
> #[export_name = "foo"] // takes precedences
> pub fn bar() {}
> ```
>
> ### Explanation
>
> The compiler will not respect the `#[no_mangle]` attribute when generating the symbol name for the function, as the `#[export_name]` attribute takes precedence. This can lead to confusion and is unnecessary.

</details>
  • Loading branch information
rust-timer authored Dec 10, 2024
2 parents b597d2a + 7951d19 commit 79254c6
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3445,6 +3445,7 @@ dependencies = [
"rustc_abi",
"rustc_arena",
"rustc_ast",
"rustc_ast_pretty",
"rustc_attr",
"rustc_data_structures",
"rustc_errors",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ regex = "1.4"
rustc_abi = { path = "../rustc_abi" }
rustc_arena = { path = "../rustc_arena" }
rustc_ast = { path = "../rustc_ast" }
rustc_ast_pretty = { path = "../rustc_ast_pretty" }
rustc_attr = { path = "../rustc_attr" }
rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ codegen_ssa_missing_memory_ordering = Atomic intrinsic missing memory ordering
codegen_ssa_missing_query_depgraph =
found CGU-reuse attribute but `-Zquery-dep-graph` was not specified
codegen_ssa_mixed_export_name_and_no_mangle = `{$no_mangle_attr}` attribute may not be used in combination with `#[export_name]`
.label = `{$no_mangle_attr}` is ignored
.note = `#[export_name]` takes precedence
.suggestion = remove the `{$no_mangle_attr}` attribute
codegen_ssa_msvc_missing_linker = the msvc targets depend on the msvc linker but `link.exe` was not found
codegen_ssa_multiple_external_func_decl = multiple declarations of external function `{$function}` from library `{$library_name}` have different calling conventions
Expand Down
57 changes: 54 additions & 3 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ use rustc_attr::{InlineAttr, InstructionSetAttr, OptimizeAttr, list_contains_nam
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::codes::*;
use rustc_errors::{DiagMessage, SubdiagMessage, struct_span_code_err};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS;
use rustc_hir::{LangItem, lang_items};
use rustc_hir::{self as hir, HirId, LangItem, lang_items};
use rustc_middle::middle::codegen_fn_attrs::{
CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry,
};
Expand Down Expand Up @@ -78,6 +77,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
let mut inline_span = None;
let mut link_ordinal_span = None;
let mut no_sanitize_span = None;
let mut mixed_export_name_no_mangle_lint_state = MixedExportNameAndNoMangleState::default();

for attr in attrs.iter() {
// In some cases, attribute are only valid on functions, but it's the `check_attr`
Expand Down Expand Up @@ -116,7 +116,12 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
sym::naked => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED,
sym::no_mangle => {
if tcx.opt_item_name(did.to_def_id()).is_some() {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE;
mixed_export_name_no_mangle_lint_state.track_no_mangle(
attr.span,
tcx.local_def_id_to_hir_id(did),
attr,
);
} else {
tcx.dcx()
.struct_span_err(
Expand Down Expand Up @@ -240,6 +245,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
.emit();
}
codegen_fn_attrs.export_name = Some(s);
mixed_export_name_no_mangle_lint_state.track_export_name(attr.span);
}
}
sym::target_feature => {
Expand Down Expand Up @@ -513,6 +519,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
}
}

mixed_export_name_no_mangle_lint_state.lint_if_mixed(tcx);

codegen_fn_attrs.inline = attrs.iter().fold(InlineAttr::None, |ia, attr| {
if !attr.has_name(sym::inline) {
return ia;
Expand Down Expand Up @@ -779,6 +787,49 @@ fn check_link_name_xor_ordinal(
}
}

#[derive(Default)]
struct MixedExportNameAndNoMangleState<'a> {
export_name: Option<Span>,
hir_id: Option<HirId>,
no_mangle: Option<Span>,
no_mangle_attr: Option<&'a ast::Attribute>,
}

impl<'a> MixedExportNameAndNoMangleState<'a> {
fn track_export_name(&mut self, span: Span) {
self.export_name = Some(span);
}

fn track_no_mangle(&mut self, span: Span, hir_id: HirId, attr_name: &'a ast::Attribute) {
self.no_mangle = Some(span);
self.hir_id = Some(hir_id);
self.no_mangle_attr = Some(attr_name);
}

/// Emit diagnostics if the lint condition is met.
fn lint_if_mixed(self, tcx: TyCtxt<'_>) {
if let Self {
export_name: Some(export_name),
no_mangle: Some(no_mangle),
hir_id: Some(hir_id),
no_mangle_attr: Some(no_mangle_attr),
} = self
{
tcx.emit_node_span_lint(
lint::builtin::UNUSED_ATTRIBUTES,
hir_id,
no_mangle,
errors::MixedExportNameAndNoMangle {
no_mangle,
no_mangle_attr: rustc_ast_pretty::pprust::attribute_to_string(no_mangle_attr),
export_name,
removal_span: no_mangle,
},
);
}
}
}

pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { codegen_fn_attrs, should_inherit_track_caller, ..*providers };
}
14 changes: 13 additions & 1 deletion compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_errors::codes::*;
use rustc_errors::{
Diag, DiagArgValue, DiagCtxtHandle, Diagnostic, EmissionGuarantee, IntoDiagArg, Level,
};
use rustc_macros::{Diagnostic, Subdiagnostic};
use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::Ty;
use rustc_middle::ty::layout::LayoutError;
use rustc_span::{Span, Symbol};
Expand Down Expand Up @@ -1114,3 +1114,15 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for TargetFeatureDisableOrEnable<'_
#[derive(Diagnostic)]
#[diag(codegen_ssa_aix_strip_not_used)]
pub(crate) struct AixStripNotUsed;

#[derive(LintDiagnostic)]
#[diag(codegen_ssa_mixed_export_name_and_no_mangle)]
pub(crate) struct MixedExportNameAndNoMangle {
#[label]
pub no_mangle: Span,
pub no_mangle_attr: String,
#[note]
pub export_name: Span,
#[suggestion(style = "verbose", code = "", applicability = "machine-applicable")]
pub removal_span: Span,
}
1 change: 0 additions & 1 deletion tests/ui/asm/naked-functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ pub unsafe extern "C" fn compatible_must_use_attributes() -> u64 {

#[export_name = "exported_function_name"]
#[link_section = ".custom_section"]
#[no_mangle]
#[naked]
pub unsafe extern "C" fn compatible_ffi_attributes_1() {
naked_asm!("", options(raw));
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/attributes/mixed_export_name_and_no_mangle.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// issue: rust-lang/rust#47446
//@ run-rustfix
//@ check-pass

#![warn(unused_attributes)]
//~^ WARN `#[no_mangle]` attribute may not be used in combination with `#[export_name]` [unused_attributes]
#[export_name = "foo"]
pub fn bar() {}

//~^ WARN `#[unsafe(no_mangle)]` attribute may not be used in combination with `#[export_name]` [unused_attributes]
#[export_name = "baz"]
pub fn bak() {}

fn main() {}
16 changes: 16 additions & 0 deletions tests/ui/attributes/mixed_export_name_and_no_mangle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// issue: rust-lang/rust#47446
//@ run-rustfix
//@ check-pass

#![warn(unused_attributes)]
#[no_mangle]
//~^ WARN `#[no_mangle]` attribute may not be used in combination with `#[export_name]` [unused_attributes]
#[export_name = "foo"]
pub fn bar() {}

#[unsafe(no_mangle)]
//~^ WARN `#[unsafe(no_mangle)]` attribute may not be used in combination with `#[export_name]` [unused_attributes]
#[export_name = "baz"]
pub fn bak() {}

fn main() {}
39 changes: 39 additions & 0 deletions tests/ui/attributes/mixed_export_name_and_no_mangle.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
warning: `#[no_mangle]` attribute may not be used in combination with `#[export_name]`
--> $DIR/mixed_export_name_and_no_mangle.rs:6:1
|
LL | #[no_mangle]
| ^^^^^^^^^^^^ `#[no_mangle]` is ignored
|
note: `#[export_name]` takes precedence
--> $DIR/mixed_export_name_and_no_mangle.rs:8:1
|
LL | #[export_name = "foo"]
| ^^^^^^^^^^^^^^^^^^^^^^
note: the lint level is defined here
--> $DIR/mixed_export_name_and_no_mangle.rs:5:9
|
LL | #![warn(unused_attributes)]
| ^^^^^^^^^^^^^^^^^
help: remove the `#[no_mangle]` attribute
|
LL - #[no_mangle]
|

warning: `#[unsafe(no_mangle)]` attribute may not be used in combination with `#[export_name]`
--> $DIR/mixed_export_name_and_no_mangle.rs:11:1
|
LL | #[unsafe(no_mangle)]
| ^^^^^^^^^^^^^^^^^^^^ `#[unsafe(no_mangle)]` is ignored
|
note: `#[export_name]` takes precedence
--> $DIR/mixed_export_name_and_no_mangle.rs:13:1
|
LL | #[export_name = "baz"]
| ^^^^^^^^^^^^^^^^^^^^^^
help: remove the `#[unsafe(no_mangle)]` attribute
|
LL - #[unsafe(no_mangle)]
|

warning: 2 warnings emitted

0 comments on commit 79254c6

Please sign in to comment.