Skip to content
Open
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
3 changes: 3 additions & 0 deletions compiler/rustc_attr_parsing/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ attr_parsing_invalid_attr_unsafe = `{$name}` is not an unsafe attribute
.suggestion = remove the `unsafe(...)`
.note = extraneous unsafe is not allowed in attributes

attr_parsing_invalid_export_visibility =
invalid export visibility: {$unrecognized_visibility}

attr_parsing_invalid_issue_string =
`issue` must be a non-zero numeric string or "none"
.must_not_be_zero = `issue` must not be "0", use "none" instead
Expand Down
40 changes: 37 additions & 3 deletions compiler/rustc_attr_parsing/src/attributes/codegen_attrs.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
use rustc_hir::attrs::{CoverageAttrKind, OptimizeAttr, RtsanSetting, SanitizerSet, UsedBy};
use std::str::FromStr;

use rustc_hir::attrs::{
CoverageAttrKind, ExportVisibilityAttrValue, OptimizeAttr, RtsanSetting, SanitizerSet, UsedBy,
};
use rustc_session::parse::feature_err;

use super::prelude::*;
use crate::session_diagnostics::{
NakedFunctionIncompatibleAttribute, NullOnExport, NullOnObjcClass, NullOnObjcSelector,
ObjcClassExpectedStringLiteral, ObjcSelectorExpectedStringLiteral,
InvalidExportVisibility, NakedFunctionIncompatibleAttribute, NullOnExport, NullOnObjcClass,
NullOnObjcSelector, ObjcClassExpectedStringLiteral, ObjcSelectorExpectedStringLiteral,
};
use crate::target_checking::Policy::AllowSilent;

Expand Down Expand Up @@ -153,6 +157,36 @@ impl<S: Stage> SingleAttributeParser<S> for ExportNameParser {
}
}

pub(crate) struct ExportVisibilityParser;

impl<S: Stage> SingleAttributeParser<S> for ExportVisibilityParser {
const PATH: &[rustc_span::Symbol] = &[sym::export_visibility];
const ATTRIBUTE_ORDER: AttributeOrder = AttributeOrder::KeepInnermost;
const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::Error;
const ALLOWED_TARGETS: AllowedTargets =
AllowedTargets::AllowListWarnRest(&[Allow(Target::Fn), Allow(Target::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 choose WarnRest here?
I'd prefer for new attributes to default to AllowedTargets::AllowList.

const TEMPLATE: AttributeTemplate = template!(NameValueStr: "visibility");

fn convert(cx: &mut AcceptContext<'_, '_, S>, args: &ArgParser) -> Option<AttributeKind> {
let Some(nv) = args.name_value() else {
cx.expected_name_value(cx.attr_span, None);
return None;
};
let Some(sv) = nv.value_as_str() else {
cx.expected_string_literal(nv.value_span, Some(nv.value_as_lit()));
return None;
};
let Ok(visibility) = ExportVisibilityAttrValue::from_str(sv.as_str()) else {
cx.emit_err(InvalidExportVisibility {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this use cx.expected_specific_argument_strings instead?
(not sure)

span: nv.value_span,
unrecognized_visibility: sv.to_string(),
});
return None;
};
Some(AttributeKind::ExportVisibility { visibility, span: cx.attr_span })
}
}

pub(crate) struct ObjcClassParser;

impl<S: Stage> SingleAttributeParser<S> for ObjcClassParser {
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_attr_parsing/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ use crate::attributes::allow_unstable::{
use crate::attributes::body::CoroutineParser;
use crate::attributes::cfi_encoding::CfiEncodingParser;
use crate::attributes::codegen_attrs::{
ColdParser, CoverageParser, EiiForeignItemParser, ExportNameParser, ForceTargetFeatureParser,
NakedParser, NoMangleParser, ObjcClassParser, ObjcSelectorParser, OptimizeParser,
RustcPassIndirectlyInNonRusticAbisParser, SanitizeParser, TargetFeatureParser,
ColdParser, CoverageParser, EiiForeignItemParser, ExportNameParser, ExportVisibilityParser,
ForceTargetFeatureParser, NakedParser, NoMangleParser, ObjcClassParser, ObjcSelectorParser,
OptimizeParser, RustcPassIndirectlyInNonRusticAbisParser, SanitizeParser, TargetFeatureParser,
ThreadLocalParser, TrackCallerParser, UsedParser,
};
use crate::attributes::confusables::ConfusablesParser;
Expand Down Expand Up @@ -209,6 +209,7 @@ attribute_parsers!(
Single<DoNotRecommendParser>,
Single<DummyParser>,
Single<ExportNameParser>,
Single<ExportVisibilityParser>,
Single<IgnoreParser>,
Single<InlineParser>,
Single<InstructionSetParser>,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_attr_parsing/src/session_diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,14 @@ pub(crate) struct UnusedMultiple {
pub name: Symbol,
}

#[derive(Diagnostic)]
#[diag(attr_parsing_invalid_export_visibility)]
pub(crate) struct InvalidExportVisibility {
#[primary_span]
pub span: Span,
pub unrecognized_visibility: String,
}

#[derive(Diagnostic)]
#[diag(attr_parsing_null_on_export, code = E0648)]
pub(crate) struct NullOnExport {
Expand Down
29 changes: 28 additions & 1 deletion compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use rustc_abi::{Align, ExternAbi};
use rustc_ast::expand::autodiff_attrs::{AutoDiffAttrs, DiffActivity, DiffMode};
use rustc_ast::{LitKind, MetaItem, MetaItemInner, attr};
use rustc_hir::attrs::{
AttributeKind, EiiImplResolution, InlineAttr, Linkage, RtsanSetting, UsedBy,
AttributeKind, EiiImplResolution, ExportVisibilityAttrValue, InlineAttr, Linkage, RtsanSetting,
UsedBy,
};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
Expand Down Expand Up @@ -132,6 +133,13 @@ fn process_builtin_attrs(
codegen_fn_attrs.inline = *inline;
interesting_spans.inline = Some(*span);
}
AttributeKind::ExportVisibility { visibility, .. } => {
codegen_fn_attrs.export_visibility = Some(match visibility {
ExportVisibilityAttrValue::TargetDefault => {
tcx.sess.default_visibility().into()
}
});
}
AttributeKind::Naked(_) => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED,
AttributeKind::Align { align, .. } => codegen_fn_attrs.alignment = Some(*align),
AttributeKind::LinkName { name, .. } => {
Expand Down Expand Up @@ -610,6 +618,25 @@ fn handle_lang_items(
}
err.emit();
}

if codegen_fn_attrs.export_visibility.is_some() {
let export_visibility_span =
find_attr!(attrs, AttributeKind::ExportVisibility{span, ..} => *span)
.unwrap_or_default();
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) {
tcx.dcx().span_err(
export_visibility_span,
"#[export_visibility = ...]` cannot be used on internal language items",
);
}
if !codegen_fn_attrs.contains_extern_indicator() {
tcx.dcx().span_err(
export_visibility_span,
"#[export_visibility = ...]` will be ignored without \
`export_name`, `no_mangle`, or similar attribute",
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make these struct errors?
In my opinion these are still nicer for simple errors like this. If you strongly disagree feel free to keep them like this

}
}
}

/// Generate the [`CodegenFnAttrs`] for an item (identified by the [`LocalDefId`]).
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
template!(NameValueStr: "name", "https://doc.rust-lang.org/reference/abi.html#the-export_name-attribute"),
FutureWarnPreceding, EncodeCrossCrate::No
),
gated!(export_visibility, Normal, template!(NameValueStr: "visibility"), ErrorPreceding, EncodeCrossCrate::No, experimental!(export_visibility)),
ungated!(
unsafe(Edition2024) link_section, Normal,
template!(NameValueStr: "name", "https://doc.rust-lang.org/reference/abi.html#the-link_section-attribute"),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,8 @@ declare_features! (
(incomplete, explicit_tail_calls, "1.72.0", Some(112788)),
/// Allows using `#[export_stable]` which indicates that an item is exportable.
(incomplete, export_stable, "1.88.0", Some(139939)),
/// Allows `#[export_visibility]` on definitions of statics and/or functions.
(unstable, export_visibility, "CURRENT_RUSTC_VERSION", Some(151425)),
/// Externally implementable items
(unstable, extern_item_impls, "CURRENT_RUSTC_VERSION", Some(125418)),
/// Allows defining `extern type`s.
Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_hir/src/attrs/data_structures.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::borrow::Cow;
use std::path::PathBuf;
use std::str::FromStr;

pub use ReprAttr::*;
use rustc_abi::Align;
Expand Down Expand Up @@ -187,6 +188,26 @@ impl Deprecation {
}
}

/// Pre-parsed value of `#[export_visibility = ...]` attribute.
///
/// In a future RFC we may consider adding support for `Hidden`, `Protected`, and/or
/// `Interposable`.
#[derive(Clone, Copy, Debug, HashStable_Generic, Encodable, Decodable, PrintAttribute)]
pub enum ExportVisibilityAttrValue {
TargetDefault,
}

impl FromStr for ExportVisibilityAttrValue {
type Err = String;

fn from_str(s: &str) -> Result<ExportVisibilityAttrValue, Self::Err> {
match s {
"target_default" => Ok(ExportVisibilityAttrValue::TargetDefault),
_ => Err(format!("'{s}' is not a valid value for #[export_visibility = ...]",)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this string used anywhere? If not, could we make the Err type ()?

}
}
}

/// There are three valid forms of the attribute:
/// `#[used]`, which is equivalent to `#[used(linker)]` on targets that support it, but `#[used(compiler)]` if not.
/// `#[used(compiler)]`
Expand Down Expand Up @@ -766,6 +787,9 @@ pub enum AttributeKind {
/// Represents `#[export_stable]`.
ExportStable,

/// Represents [`#[export_visibility = ...]`](https://github.com/rust-lang/rust/issues/151425)
ExportVisibility { visibility: ExportVisibilityAttrValue, span: Span },

/// Represents `#[ffi_const]`.
FfiConst(Span),

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/attrs/encode_cross_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ impl AttributeKind {
EiiImpls(..) => No,
ExportName { .. } => Yes,
ExportStable => No,
ExportVisibility { .. } => Yes,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is exporting this attribute needed? (Not sure, genuine question)

FfiConst(..) => No,
FfiPure(..) => No,
Fundamental { .. } => Yes,
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ pub struct CodegenFnAttrs {
/// be set when `link_name` is set. This is for foreign items with the
/// "raw-dylib" kind.
pub link_ordinal: Option<u16>,
/// The `#[export_visibility = "..."]` attribute, with values interpreted
/// as follows:
/// * `None` - use the "inherent" visibility (either based on the target platform, or provided via
/// `-Zdefault-visibility=...` command-line flag)
/// * `Some(...)` - use the item/symbol-specific visibility
pub export_visibility: Option<Visibility>,
/// The `#[target_feature(enable = "...")]` attribute and the enabled
/// features (only enabled features are supported right now).
/// Implied target features have already been applied.
Expand Down Expand Up @@ -224,6 +230,7 @@ impl CodegenFnAttrs {
optimize: OptimizeAttr::Default,
symbol_name: None,
link_ordinal: None,
export_visibility: None,
target_features: vec![],
foreign_item_symbol_aliases: vec![],
safe_target_features: false,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_monomorphize/src/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,11 @@ fn mono_item_visibility<'tcx>(
}

fn default_visibility(tcx: TyCtxt<'_>, id: DefId, is_generic: bool) -> Visibility {
// If present, then symbol-specific `#[export_visibility = ...]` "wins".
if let Some(visibility) = tcx.codegen_fn_attrs(id).export_visibility {
return visibility;
}

// Fast-path to avoid expensive query call below
if tcx.sess.default_visibility() == SymbolVisibility::Interposable {
return Visibility::Default;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
| AttributeKind::Repr { .. }
| AttributeKind::Cold(..)
| AttributeKind::ExportName { .. }
| AttributeKind::ExportVisibility { .. }
| AttributeKind::Fundamental
| AttributeKind::Optimize(..)
| AttributeKind::LinkSection { .. }
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,7 @@ symbols! {
explicit_tail_calls,
export_name,
export_stable,
export_visibility,
expr,
expr_2021,
expr_fragment_specifier_2024,
Expand Down
92 changes: 92 additions & 0 deletions tests/codegen-llvm/export-visibility.rs
Copy link
Contributor

Choose a reason for hiding this comment

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

Because of how complex and target-specific everything linker-related is, I think an end-to-end test in tests/run-make would be great to cover an ELF/PE/Mach-O target. Then you have access to the object crate via run_make_support for checking symbol visibility.

To allow each object file type to be built on all platforms with just --target, the test can use a minicore like

(unfortunately there is nothing convenient like what codegen tests have with //@ add-minicore).

Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Verifies that `#[export_visibility = ...]` can override the visibility
// that is normally implied by `#[export_name]` or `#[no_mangle]`.
//
// High-level test expectations for items with `#[export_name = ...]`
// (or with `#[no_mangle]`) and:
//
// * Without `#[export_visibility = ...]` => public
// * `#[export_visibility = "target_default"]` => value inherited from the target
// platform or from the `-Zdefault-visibility=...` command-line flag
// (this expectation depends on whether the `HIDDEN` vs `PROTECTED`
// test revision is used).
//
// Note that what we call "public" in the expectations above is also referred
// to as "default" in LLVM docs - see
// https://llvm.org/docs/LangRef.html#visibility-styles

//@ revisions:HIDDEN PROTECTED
//@[HIDDEN] compile-flags: -Zdefault-visibility=hidden
//@[PROTECTED] compile-flags: -Zdefault-visibility=protected

// This test focuses on rlib to exercise the scenario described in
// https://github.com/rust-lang/rust/issues/73958#issuecomment-2891711649
#![crate_type = "rlib"]
#![feature(export_visibility)]

// Exact LLVM IR differs depending on the target triple (e.g. `hidden constant`
// vs `internal constant` vs `constant`). Because of this, we only apply the
// specific test expectations below to one specific target triple. If needed,
// additional targets can be covered by adding copies of this test file with
// a different `only-X` directive.
//
//@ only-x86_64-unknown-linux-gnu
Comment on lines +26 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to use add-minicore here so it gets checked on all targets:

//@ add-minicore
//@ compile-flags: --target x86_64-unknown-linux-gnu

#![feature(no_core)]
#![no_core]

Possibly also roll this into the revisions so different targets wouldn't need different files.

//@ revisions: LINUX-X86-HIDDEN LINUX-X86-PROTECTED
//@ [LINUX-X86-HIDDEN,LINUX-X86-PROTECTED] compile-flags: --target x86_64-unknown-linux-gnu
...


///////////////////////////////////////////////////////////////////////
// The tests below focus on how `#[export_visibility = ...]` works for
// a `static`. The tests are based on similar tests in
// `tests/codegen/default-visibility.rs`

#[unsafe(export_name = "test_static_no_attr")]
pub static TEST_STATIC_NO_ATTR: [u8; 7] = *b"static1";

#[unsafe(export_name = "test_static_target_default")]
#[export_visibility = "target_default"]
pub static TESTED_STATIC_ATTR_ASKS_TO_TARGET_DEFAULT: [u8; 7] = *b"static2";

#[unsafe(no_mangle)]
pub static test_static_no_mangle_no_attr: [u8; 10] = *b"no_mangle1";

#[unsafe(no_mangle)]
#[export_visibility = "target_default"]
pub static test_static_no_mangle_target_default: [u8; 10] = *b"no_mangle2";

// HIDDEN: @test_static_no_attr = local_unnamed_addr constant
// HIDDEN: @test_static_target_default = hidden local_unnamed_addr constant
// HIDDEN: @test_static_no_mangle_no_attr = local_unnamed_addr constant
// HIDDEN: @test_static_no_mangle_target_default = hidden local_unnamed_addr constant

// PROTECTED: @test_static_no_attr = local_unnamed_addr constant
// PROTECTED: @test_static_target_default = protected local_unnamed_addr constant
// PROTECTED: @test_static_no_mangle_no_attr = local_unnamed_addr constant
// PROTECTED: @test_static_no_mangle_target_default = protected local_unnamed_addr constant

///////////////////////////////////////////////////////////////////////
// The tests below focus on how `#[export_visibility = ...]` works for
// a `fn`.
//
// The tests below try to mimics how `cxx` exports known/hardcoded helpers (e.g.
// `cxxbridge1$string$drop` [1]) as well as build-time-generated thunks (e.g.
// `serde_json_lenient$cxxbridge1$decode_json` from https://crbug.com/418073233#comment7).
//
// We use `line!()` to ensure that each function has a unique body
// (and therefore that the functions won't get folded together).
//
// [1]
// https://github.com/dtolnay/cxx/blob/ebdd6a0c63ae10dc5224ed21970b7a0504657434/src/symbols/rust_string.rs#L83-L86

#[unsafe(export_name = "test_fn_no_attr")]
unsafe extern "C" fn test_fn_no_attr() -> u32 {
line!()
}

#[unsafe(export_name = "test_fn_target_default")]
#[export_visibility = "target_default"]
unsafe extern "C" fn test_fn_asks_for_target_default() -> u32 {
line!()
}

// HIDDEN: define noundef i32 @test_fn_no_attr
// HIDDEN: define hidden noundef i32 @test_fn_target_default

// PROTECTED: define noundef i32 @test_fn_no_attr
// PROTECTED: define protected noundef i32 @test_fn_target_default
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// This test verfies that `#[export_visibility = ...]` will report an error
// when applied to an item that also has `#[rustc_std_internal_symbol]`
// attribute.
#![feature(export_visibility)]
#![feature(rustc_attrs)]
#[export_visibility = "target_default"]
//~^ERROR: #[export_visibility = ...]` cannot be used on internal language items
#[rustc_std_internal_symbol]
pub static TESTED_STATIC: [u8; 6] = *b"foobar";

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: #[export_visibility = ...]` cannot be used on internal language items
--> $DIR/export-visibility-with-rustc-std-internal-symbol.rs:6:1
|
LL | #[export_visibility = "target_default"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

Loading
Loading