From d8344ff5b551e668439d579517e8fc78f7f6dfe2 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sat, 9 Feb 2019 20:09:56 +0100 Subject: [PATCH 01/11] Add ffi_const and ffi_pure extern fn attributes --- src/librustc/hir/mod.rs | 23 ++++++++++++++++++ src/librustc_codegen_llvm/attributes.rs | 6 +++++ src/librustc_codegen_llvm/llvm/ffi.rs | 1 + src/librustc_typeck/collect.rs | 24 +++++++++++++++++++ src/librustc_typeck/diagnostics.rs | 2 ++ src/libsyntax/feature_gate.rs | 16 +++++++++++++ src/rustllvm/RustWrapper.cpp | 2 ++ src/rustllvm/rustllvm.h | 1 + src/test/codegen/ffi-const.rs | 16 +++++++++++++ src/test/codegen/ffi-pure.rs | 13 ++++++++++ .../feature-gates/feature-gate-ffi_const.rs | 7 ++++++ .../feature-gate-ffi_const.stderr | 11 +++++++++ .../ui/feature-gates/feature-gate-ffi_pure.rs | 7 ++++++ .../feature-gate-ffi_pure.stderr | 11 +++++++++ src/test/ui/ffi_const.rs | 6 +++++ src/test/ui/ffi_const.stderr | 9 +++++++ src/test/ui/ffi_pure.rs | 6 +++++ src/test/ui/ffi_pure.stderr | 9 +++++++ 18 files changed, 170 insertions(+) create mode 100644 src/test/codegen/ffi-const.rs create mode 100644 src/test/codegen/ffi-pure.rs create mode 100644 src/test/ui/feature-gates/feature-gate-ffi_const.rs create mode 100644 src/test/ui/feature-gates/feature-gate-ffi_const.stderr create mode 100644 src/test/ui/feature-gates/feature-gate-ffi_pure.rs create mode 100644 src/test/ui/feature-gates/feature-gate-ffi_pure.stderr create mode 100644 src/test/ui/ffi_const.rs create mode 100644 src/test/ui/ffi_const.stderr create mode 100644 src/test/ui/ffi_pure.rs create mode 100644 src/test/ui/ffi_pure.stderr diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 3e7dd1432e1e3..5eeceab593342 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2489,6 +2489,29 @@ bitflags! { /// #[used], indicates that LLVM can't eliminate this function (but the /// linker can!) const USED = 1 << 9; + /// #[ffi_pure], indicates that a function has no effects except for + /// its return value, that its return value depends only on the + /// parameters and/or global variables, and that its return value does + /// not change between two consecutive calls (for example, because of + /// volatile access to the global variables). + /// + /// Such a function can be subject to common sub-expression elimination + /// and loop optimization just as an arithmetic operator would be. Some + /// common examples of pure functions are `strlen` or `memcmp`. + const FFI_PURE = 1 << 10; + /// #[ffi_const], indicates that a function has no effects except for + /// its return value and that its return value depends only on the + /// value of the function parameters. + /// + /// This attribute is stricter than `#[ffi_pure]`, since the function + /// return value is not allowed to depend on anything that is not the + /// value of the function parameters, like global memory, or other + /// values, like dereferencing a pointer function parameter to read + /// another value. + /// + /// A `#[ffi_const]` function that returns void is a `nop` in the + /// abstract machine. + const FFI_CONST = 1 << 11; } } diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index e6bc7bca46bc9..8ca891a51aa7c 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -223,6 +223,12 @@ pub fn from_fn_attrs( if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::COLD) { Attribute::Cold.apply_llfn(Function, llfn); } + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::FFI_PURE) { + Attribute::ReadOnly.apply_llfn(Function, llfn); + } + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::FFI_CONST) { + Attribute::ReadNone.apply_llfn(Function, llfn); + } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { naked(llfn, true); } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 58bdfc47fcaed..569850e350eb3 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -116,6 +116,7 @@ pub enum Attribute { SanitizeMemory = 22, NonLazyBind = 23, OptimizeNone = 24, + ReadNone = 25, } /// LLVMIntPredicate diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 9dc74c5d63a4e..78d8da01c97b2 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2270,6 +2270,30 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR; } else if attr.check_name("unwind") { codegen_fn_attrs.flags |= CodegenFnAttrFlags::UNWIND; + } else if attr.check_name("ffi_pure") { + if tcx.is_foreign_item(id) { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_PURE; + } else { + // `#[ffi_pure]` is only allowed `extern fn`s + struct_span_err!( + tcx.sess, + attr.span, + E0724, + "`#[ffi_pure]` may only be used on `extern fn`s" + ).emit(); + } + } else if attr.check_name("ffi_const") { + if tcx.is_foreign_item(id) { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_CONST; + } else { + // `#[ffi_const]` is only allowed `extern fn`s + struct_span_err!( + tcx.sess, + attr.span, + E0725, + "`#[ffi_const]` may only be used on `extern fn`s" + ).emit(); + } } else if attr.check_name("rustc_allocator_nounwind") { codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND; } else if attr.check_name("naked") { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 3ed09dfe99239..93127b7f7f60d 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -4720,4 +4720,6 @@ register_diagnostics! { E0698, // type inside generator must be known in this context E0719, // duplicate values for associated type binding E0722, // Malformed #[optimize] attribute + E0724, // `#[ffi_pure]` is only allowed `extern fn`s + E0725, // `#[ffi_const]` is only allowed `extern fn`s } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index e7b9a884b5e0c..ce8ededa71771 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -290,6 +290,12 @@ declare_features! ( // The `repr(i128)` annotation for enums. (active, repr128, "1.16.0", Some(35118), None), + // Allows the use of `#[ffi_pure]` on extern functions. + (active, ffi_pure, "1.34.0", Some(58329), None), + + // Allows the use of `#[ffi_const]` on extern functions. + (active, ffi_const, "1.34.0", Some(58328), None), + // The `unadjusted` ABI; perma-unstable. // // rustc internal @@ -1124,6 +1130,16 @@ pub const BUILTIN_ATTRIBUTES: &[(&str, AttributeType, AttributeTemplate, Attribu "the `#[naked]` attribute \ is an experimental feature", cfg_fn!(naked_functions))), + ("ffi_pure", Whitelisted, template!(Word), Gated(Stability::Unstable, + "ffi_pure", + "the `#[ffi_pure]` attribute \ + is an experimental feature", + cfg_fn!(ffi_pure))), + ("ffi_const", Whitelisted, template!(Word), Gated(Stability::Unstable, + "ffi_const", + "the `#[ffi_const]` attribute \ + is an experimental feature", + cfg_fn!(ffi_const))), ("target_feature", Whitelisted, template!(List: r#"enable = "name""#), Ungated), ("export_name", Whitelisted, template!(NameValueStr: "name"), Ungated), ("inline", Whitelisted, template!(Word, List: "always|never"), Ungated), diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index b33165b846339..c87b78ba4d5e3 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -170,6 +170,8 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { return Attribute::OptimizeForSize; case ReadOnly: return Attribute::ReadOnly; + case ReadNone: + return Attribute::ReadNone; case SExt: return Attribute::SExt; case StructRet: diff --git a/src/rustllvm/rustllvm.h b/src/rustllvm/rustllvm.h index 933266b402526..2ea57e46bcd45 100644 --- a/src/rustllvm/rustllvm.h +++ b/src/rustllvm/rustllvm.h @@ -85,6 +85,7 @@ enum LLVMRustAttribute { SanitizeMemory = 22, NonLazyBind = 23, OptimizeNone = 24, + ReadNone = 25, }; typedef struct OpaqueRustString *RustStringRef; diff --git a/src/test/codegen/ffi-const.rs b/src/test/codegen/ffi-const.rs new file mode 100644 index 0000000000000..3574f776e3a6b --- /dev/null +++ b/src/test/codegen/ffi-const.rs @@ -0,0 +1,16 @@ +// compile-flags: -C no-prepopulate-passes +#![crate_type = "lib"] +#![feature(ffi_const)] + +// CHECK-LABEL: @bar() +#[no_mangle] +pub fn bar() { unsafe { foo() } } + + +extern { + // CHECK-LABEL: @foo() unnamed_addr #1 + // CHECK: attributes #1 = { {{.*}}readnone{{.*}} } + #[no_mangle] + #[ffi_const] + pub fn foo(); +} diff --git a/src/test/codegen/ffi-pure.rs b/src/test/codegen/ffi-pure.rs new file mode 100644 index 0000000000000..614e001c7a8d8 --- /dev/null +++ b/src/test/codegen/ffi-pure.rs @@ -0,0 +1,13 @@ +// compile-flags: -C no-prepopulate-passes +#![crate_type = "lib"] +#![feature(ffi_pure)] + +// CHECK-LABEL: @bar() +#[no_mangle] +pub fn bar() { unsafe { foo() } } + +extern { + // CHECK-LABEL: @foo() unnamed_addr #1 + // CHECK: attributes #1 = { {{.*}}readonly{{.*}} } + #[ffi_pure] pub fn foo(); +} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_const.rs b/src/test/ui/feature-gates/feature-gate-ffi_const.rs new file mode 100644 index 0000000000000..98ae22655be83 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-ffi_const.rs @@ -0,0 +1,7 @@ +// ignore-tidy-linelength +#![crate_type = "lib"] + +extern { + #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58314) + pub fn foo(); +} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_const.stderr b/src/test/ui/feature-gates/feature-gate-ffi_const.stderr new file mode 100644 index 0000000000000..43bb21daa9382 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-ffi_const.stderr @@ -0,0 +1,11 @@ +error[E0658]: the `#[ffi_const]` attribute is an experimental feature (see issue #58314) + --> $DIR/feature-gate-ffi_const.rs:5:5 + | +LL | #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58314) + | ^^^^^^^^^^^^ + | + = help: add #![feature(ffi_const)] to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/feature-gates/feature-gate-ffi_pure.rs b/src/test/ui/feature-gates/feature-gate-ffi_pure.rs new file mode 100644 index 0000000000000..d25c1d1317d27 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-ffi_pure.rs @@ -0,0 +1,7 @@ +// ignore-tidy-linelength +#![crate_type = "lib"] + +extern { + #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58314) + pub fn foo(); +} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr b/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr new file mode 100644 index 0000000000000..bcc97a454adb2 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr @@ -0,0 +1,11 @@ +error[E0658]: the `#[ffi_pure]` attribute is an experimental feature (see issue #58314) + --> $DIR/feature-gate-ffi_pure.rs:5:5 + | +LL | #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58314) + | ^^^^^^^^^^^ + | + = help: add #![feature(ffi_pure)] to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/ffi_const.rs b/src/test/ui/ffi_const.rs new file mode 100644 index 0000000000000..322d8d869753c --- /dev/null +++ b/src/test/ui/ffi_const.rs @@ -0,0 +1,6 @@ +// ignore-tidy-linelength +#![feature(ffi_const)] +#![crate_type = "lib"] + +#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on `extern fn`s [E0725] +pub fn foo() {} diff --git a/src/test/ui/ffi_const.stderr b/src/test/ui/ffi_const.stderr new file mode 100644 index 0000000000000..e7fdf333e9e5e --- /dev/null +++ b/src/test/ui/ffi_const.stderr @@ -0,0 +1,9 @@ +error[E0725]: `#[ffi_const]` may only be used on `extern fn`s + --> $DIR/ffi_const.rs:5:1 + | +LL | #[ffi_const] //~ ERROR `#[ffi_const]` may only be used on `extern fn`s [E0725] + | ^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0725`. diff --git a/src/test/ui/ffi_pure.rs b/src/test/ui/ffi_pure.rs new file mode 100644 index 0000000000000..01c32114e5547 --- /dev/null +++ b/src/test/ui/ffi_pure.rs @@ -0,0 +1,6 @@ +// ignore-tidy-linelength +#![feature(ffi_pure)] +#![crate_type = "lib"] + +#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on `extern fn`s [E0724] +pub fn foo() {} diff --git a/src/test/ui/ffi_pure.stderr b/src/test/ui/ffi_pure.stderr new file mode 100644 index 0000000000000..618ccbdf6da45 --- /dev/null +++ b/src/test/ui/ffi_pure.stderr @@ -0,0 +1,9 @@ +error[E0724]: `#[ffi_pure]` may only be used on `extern fn`s + --> $DIR/ffi_pure.rs:5:1 + | +LL | #[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on `extern fn`s [E0724] + | ^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0724`. From bf149c0cbf7c64f0c89abb5f16a4c979b01c2c57 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sat, 9 Feb 2019 20:55:54 +0100 Subject: [PATCH 02/11] Use the term foreign function in errors --- src/librustc_typeck/collect.rs | 4 ++-- src/librustc_typeck/diagnostics.rs | 4 ++-- src/libsyntax/feature_gate.rs | 4 ++-- src/test/ui/feature-gates/feature-gate-ffi_const.rs | 2 +- src/test/ui/feature-gates/feature-gate-ffi_const.stderr | 4 ++-- src/test/ui/feature-gates/feature-gate-ffi_pure.rs | 2 +- src/test/ui/feature-gates/feature-gate-ffi_pure.stderr | 4 ++-- src/test/ui/ffi_const.rs | 2 +- src/test/ui/ffi_const.stderr | 4 ++-- src/test/ui/ffi_pure.rs | 2 +- src/test/ui/ffi_pure.stderr | 4 ++-- 11 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 78d8da01c97b2..cfe409c37e2e1 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2279,7 +2279,7 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen tcx.sess, attr.span, E0724, - "`#[ffi_pure]` may only be used on `extern fn`s" + "`#[ffi_pure]` may only be used on foreign functions" ).emit(); } } else if attr.check_name("ffi_const") { @@ -2291,7 +2291,7 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen tcx.sess, attr.span, E0725, - "`#[ffi_const]` may only be used on `extern fn`s" + "`#[ffi_const]` may only be used on foreign functions" ).emit(); } } else if attr.check_name("rustc_allocator_nounwind") { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 93127b7f7f60d..aa8a59791a7ba 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -4720,6 +4720,6 @@ register_diagnostics! { E0698, // type inside generator must be known in this context E0719, // duplicate values for associated type binding E0722, // Malformed #[optimize] attribute - E0724, // `#[ffi_pure]` is only allowed `extern fn`s - E0725, // `#[ffi_const]` is only allowed `extern fn`s + E0724, // `#[ffi_pure]` is only allowed on foreign fn + E0725, // `#[ffi_const]` is only allowed on foreign fn } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index ce8ededa71771..39fe5fa7516e7 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -290,10 +290,10 @@ declare_features! ( // The `repr(i128)` annotation for enums. (active, repr128, "1.16.0", Some(35118), None), - // Allows the use of `#[ffi_pure]` on extern functions. + // Allows the use of `#[ffi_pure]` on foreign functions. (active, ffi_pure, "1.34.0", Some(58329), None), - // Allows the use of `#[ffi_const]` on extern functions. + // Allows the use of `#[ffi_const]` on foreign functions. (active, ffi_const, "1.34.0", Some(58328), None), // The `unadjusted` ABI; perma-unstable. diff --git a/src/test/ui/feature-gates/feature-gate-ffi_const.rs b/src/test/ui/feature-gates/feature-gate-ffi_const.rs index 98ae22655be83..7891fea430468 100644 --- a/src/test/ui/feature-gates/feature-gate-ffi_const.rs +++ b/src/test/ui/feature-gates/feature-gate-ffi_const.rs @@ -2,6 +2,6 @@ #![crate_type = "lib"] extern { - #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58314) + #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58328) pub fn foo(); } diff --git a/src/test/ui/feature-gates/feature-gate-ffi_const.stderr b/src/test/ui/feature-gates/feature-gate-ffi_const.stderr index 43bb21daa9382..62852fc6510f7 100644 --- a/src/test/ui/feature-gates/feature-gate-ffi_const.stderr +++ b/src/test/ui/feature-gates/feature-gate-ffi_const.stderr @@ -1,7 +1,7 @@ -error[E0658]: the `#[ffi_const]` attribute is an experimental feature (see issue #58314) +error[E0658]: the `#[ffi_const]` attribute is an experimental feature (see issue #58328) --> $DIR/feature-gate-ffi_const.rs:5:5 | -LL | #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58314) +LL | #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58328) | ^^^^^^^^^^^^ | = help: add #![feature(ffi_const)] to the crate attributes to enable diff --git a/src/test/ui/feature-gates/feature-gate-ffi_pure.rs b/src/test/ui/feature-gates/feature-gate-ffi_pure.rs index d25c1d1317d27..9db77afda6c84 100644 --- a/src/test/ui/feature-gates/feature-gate-ffi_pure.rs +++ b/src/test/ui/feature-gates/feature-gate-ffi_pure.rs @@ -2,6 +2,6 @@ #![crate_type = "lib"] extern { - #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58314) + #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58329) pub fn foo(); } diff --git a/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr b/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr index bcc97a454adb2..9108e13afb91c 100644 --- a/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr +++ b/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr @@ -1,7 +1,7 @@ -error[E0658]: the `#[ffi_pure]` attribute is an experimental feature (see issue #58314) +error[E0658]: the `#[ffi_pure]` attribute is an experimental feature (see issue #58329) --> $DIR/feature-gate-ffi_pure.rs:5:5 | -LL | #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58314) +LL | #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58329) | ^^^^^^^^^^^ | = help: add #![feature(ffi_pure)] to the crate attributes to enable diff --git a/src/test/ui/ffi_const.rs b/src/test/ui/ffi_const.rs index 322d8d869753c..11554ffb3285d 100644 --- a/src/test/ui/ffi_const.rs +++ b/src/test/ui/ffi_const.rs @@ -2,5 +2,5 @@ #![feature(ffi_const)] #![crate_type = "lib"] -#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on `extern fn`s [E0725] +#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions [E0725] pub fn foo() {} diff --git a/src/test/ui/ffi_const.stderr b/src/test/ui/ffi_const.stderr index e7fdf333e9e5e..f74c7787bab0a 100644 --- a/src/test/ui/ffi_const.stderr +++ b/src/test/ui/ffi_const.stderr @@ -1,7 +1,7 @@ -error[E0725]: `#[ffi_const]` may only be used on `extern fn`s +error[E0725]: `#[ffi_const]` may only be used on foreign functions --> $DIR/ffi_const.rs:5:1 | -LL | #[ffi_const] //~ ERROR `#[ffi_const]` may only be used on `extern fn`s [E0725] +LL | #[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions [E0725] | ^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/ffi_pure.rs b/src/test/ui/ffi_pure.rs index 01c32114e5547..29d4142d8f25a 100644 --- a/src/test/ui/ffi_pure.rs +++ b/src/test/ui/ffi_pure.rs @@ -2,5 +2,5 @@ #![feature(ffi_pure)] #![crate_type = "lib"] -#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on `extern fn`s [E0724] +#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions [E0724] pub fn foo() {} diff --git a/src/test/ui/ffi_pure.stderr b/src/test/ui/ffi_pure.stderr index 618ccbdf6da45..11f134ce95382 100644 --- a/src/test/ui/ffi_pure.stderr +++ b/src/test/ui/ffi_pure.stderr @@ -1,7 +1,7 @@ -error[E0724]: `#[ffi_pure]` may only be used on `extern fn`s +error[E0724]: `#[ffi_pure]` may only be used on foreign functions --> $DIR/ffi_pure.rs:5:1 | -LL | #[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on `extern fn`s [E0724] +LL | #[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions [E0724] | ^^^^^^^^^^^ error: aborting due to previous error From 945cbe43235bf415d26701f246728f021a077b1d Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sun, 10 Feb 2019 10:01:05 +0100 Subject: [PATCH 03/11] Use pattern to match attributes --- src/test/codegen/ffi-const.rs | 11 +++-------- src/test/codegen/ffi-pure.rs | 6 ++---- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/test/codegen/ffi-const.rs b/src/test/codegen/ffi-const.rs index 3574f776e3a6b..2e1cd5c812495 100644 --- a/src/test/codegen/ffi-const.rs +++ b/src/test/codegen/ffi-const.rs @@ -2,15 +2,10 @@ #![crate_type = "lib"] #![feature(ffi_const)] -// CHECK-LABEL: @bar() -#[no_mangle] pub fn bar() { unsafe { foo() } } - extern { - // CHECK-LABEL: @foo() unnamed_addr #1 - // CHECK: attributes #1 = { {{.*}}readnone{{.*}} } - #[no_mangle] - #[ffi_const] - pub fn foo(); + #[ffi_const] pub fn foo(); } +// CHECK: declare void @foo(){{.*}}#1{{.*}} +// CHECK: attributes #1 = { {{.*}}readnone{{.*}} } diff --git a/src/test/codegen/ffi-pure.rs b/src/test/codegen/ffi-pure.rs index 614e001c7a8d8..69bd77943d7de 100644 --- a/src/test/codegen/ffi-pure.rs +++ b/src/test/codegen/ffi-pure.rs @@ -2,12 +2,10 @@ #![crate_type = "lib"] #![feature(ffi_pure)] -// CHECK-LABEL: @bar() -#[no_mangle] pub fn bar() { unsafe { foo() } } extern { - // CHECK-LABEL: @foo() unnamed_addr #1 - // CHECK: attributes #1 = { {{.*}}readonly{{.*}} } #[ffi_pure] pub fn foo(); } +// CHECK: declare void @foo(){{.*}}#1{{.*}} +// CHECK: attributes #1 = { {{.*}}readonly{{.*}} } From 736c437f494755956e82ebe49a7469baba7f3c29 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sun, 10 Feb 2019 12:02:55 +0100 Subject: [PATCH 04/11] Fix attribute check --- src/test/codegen/ffi-const.rs | 5 +++-- src/test/codegen/ffi-pure.rs | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/test/codegen/ffi-const.rs b/src/test/codegen/ffi-const.rs index 2e1cd5c812495..440d022a12cba 100644 --- a/src/test/codegen/ffi-const.rs +++ b/src/test/codegen/ffi-const.rs @@ -5,7 +5,8 @@ pub fn bar() { unsafe { foo() } } extern { + // CHECK-LABEL: declare void @foo() + // CHECK-SAME: [[ATTRS:#[0-9]+]] + // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}readnone{{.*}} } #[ffi_const] pub fn foo(); } -// CHECK: declare void @foo(){{.*}}#1{{.*}} -// CHECK: attributes #1 = { {{.*}}readnone{{.*}} } diff --git a/src/test/codegen/ffi-pure.rs b/src/test/codegen/ffi-pure.rs index 69bd77943d7de..45aad4aeb3d1e 100644 --- a/src/test/codegen/ffi-pure.rs +++ b/src/test/codegen/ffi-pure.rs @@ -5,7 +5,10 @@ pub fn bar() { unsafe { foo() } } extern { + // CHECK-LABEL: declare void @foo() + // CHECK-SAME: [[ATTRS:#[0-9]+]] + // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}readonly{{.*}} } #[ffi_pure] pub fn foo(); } -// CHECK: declare void @foo(){{.*}}#1{{.*}} -// CHECK: attributes #1 = { {{.*}}readonly{{.*}} } + + From 3adedddeac1ee650081b3e7528389bd4a95c1e11 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sun, 10 Feb 2019 12:48:26 +0100 Subject: [PATCH 05/11] Fix tidy --- src/test/codegen/ffi-pure.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/codegen/ffi-pure.rs b/src/test/codegen/ffi-pure.rs index 45aad4aeb3d1e..f0ebc1caa09bd 100644 --- a/src/test/codegen/ffi-pure.rs +++ b/src/test/codegen/ffi-pure.rs @@ -10,5 +10,3 @@ extern { // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}readonly{{.*}} } #[ffi_pure] pub fn foo(); } - - From effaf14f04d13f52b098ab2671471e2cc289c83d Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 11 Feb 2019 15:12:25 +0100 Subject: [PATCH 06/11] Update doc of `ffi_const` Co-Authored-By: gnzlbg --- src/librustc/hir/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 5eeceab593342..8ce6394091c1a 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2509,7 +2509,7 @@ bitflags! { /// values, like dereferencing a pointer function parameter to read /// another value. /// - /// A `#[ffi_const]` function that returns void is a `nop` in the + /// A `#[ffi_const]` function that returns a unit type has no effect on the abstract machine's state. /// abstract machine. const FFI_CONST = 1 << 11; } From 26eb8b0fc228cec48212369331ca085b6c5f1c42 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 12 Feb 2019 01:00:59 +0100 Subject: [PATCH 07/11] Document in unstable book; check that const functions are not pure, rename to c_ffi_... --- .../src/language-features/c_ffi_const.md | 51 +++++++++++++++++ .../src/language-features/c_ffi_pure.md | 55 +++++++++++++++++++ src/librustc/hir/mod.rs | 29 ++-------- src/librustc_codegen_llvm/attributes.rs | 4 +- src/librustc_typeck/collect.rs | 25 ++++++--- src/librustc_typeck/diagnostics.rs | 5 +- src/libsyntax/feature_gate.rs | 24 ++++---- .../codegen/{ffi-const.rs => c-ffi-const.rs} | 4 +- .../codegen/{ffi-pure.rs => c-ffi-pure.rs} | 4 +- src/test/ui/c_ffi_const.rs | 10 ++++ .../{ffi_const.stderr => c_ffi_const.stderr} | 0 src/test/ui/c_ffi_pure.rs | 6 ++ .../ui/{ffi_pure.stderr => c_ffi_pure.stderr} | 0 .../feature-gates/feature-gate-c_ffi_const.rs | 7 +++ ...stderr => feature-gate-c_ffi_const.stderr} | 0 .../feature-gates/feature-gate-c_ffi_pure.rs | 7 +++ ....stderr => feature-gate-c_ffi_pure.stderr} | 0 .../feature-gates/feature-gate-ffi_const.rs | 7 --- .../ui/feature-gates/feature-gate-ffi_pure.rs | 7 --- src/test/ui/ffi_const.rs | 6 -- src/test/ui/ffi_pure.rs | 6 -- 21 files changed, 180 insertions(+), 77 deletions(-) create mode 100644 src/doc/unstable-book/src/language-features/c_ffi_const.md create mode 100644 src/doc/unstable-book/src/language-features/c_ffi_pure.md rename src/test/codegen/{ffi-const.rs => c-ffi-const.rs} (81%) rename src/test/codegen/{ffi-pure.rs => c-ffi-pure.rs} (82%) create mode 100644 src/test/ui/c_ffi_const.rs rename src/test/ui/{ffi_const.stderr => c_ffi_const.stderr} (100%) create mode 100644 src/test/ui/c_ffi_pure.rs rename src/test/ui/{ffi_pure.stderr => c_ffi_pure.stderr} (100%) create mode 100644 src/test/ui/feature-gates/feature-gate-c_ffi_const.rs rename src/test/ui/feature-gates/{feature-gate-ffi_const.stderr => feature-gate-c_ffi_const.stderr} (100%) create mode 100644 src/test/ui/feature-gates/feature-gate-c_ffi_pure.rs rename src/test/ui/feature-gates/{feature-gate-ffi_pure.stderr => feature-gate-c_ffi_pure.stderr} (100%) delete mode 100644 src/test/ui/feature-gates/feature-gate-ffi_const.rs delete mode 100644 src/test/ui/feature-gates/feature-gate-ffi_pure.rs delete mode 100644 src/test/ui/ffi_const.rs delete mode 100644 src/test/ui/ffi_pure.rs diff --git a/src/doc/unstable-book/src/language-features/c_ffi_const.md b/src/doc/unstable-book/src/language-features/c_ffi_const.md new file mode 100644 index 0000000000000..db6b1a5883b92 --- /dev/null +++ b/src/doc/unstable-book/src/language-features/c_ffi_const.md @@ -0,0 +1,51 @@ +# `c_ffi_const` + +The `#[c_ffi_const]` attribute applies clang's `const` attribute to foreign +functions declarations. + +That is, `#[c_ffi_const]` functions shall have no effects except for its return +value, which can only depend on the values of the function parameters, and is +not affected by changes to the observable state of the program. + +The behavior of calling a `#[c_ffi_const]` function that violates these +requirements is undefined. + +This attribute enables Rust to perform common optimizations, like sub-expression +elimination, and it can avoid emitting some calls in repeated invocations of the +function with the same argument values regardless of other operations being +performed in between these functions calls (as opposed to `#[c_ffi_pure]` +functions). + +## Pitfals + +A `#[c_ffi_const]` function can only read global memory that would not affect +its return value for the whole execution of the program (e.g. immutable global +memory). `#[c_ffi_const]` functions are referentially-transparent and therefore +more strict than `#[c_ffi_pure]` functions. + +A common pitfall involves applying the `#[c_ffi_const]` attribute to a +function that reads memory through pointer arguments which do not necessarily +point to immutable global memory. + +A `#[c_ffi_const]` function that returns unit has no effect on the abstract +machine's state, and a `#[c_ffi_const]` function cannot be `#[c_ffi_pure]`. + +A diverging and C or C++ `const` function is unreachable. Diverging via a +side-effect (e.g. a call to `abort`) violates `const` pre-conditions. Divergence +without side-effects is undefined behavior in C++ and not possible in C. In C++, +the behavior of infinite loops without side-effects is undefined, while in C +these loops can be assumed to terminate. This would cause a diverging function +to return, invoking undefined behavior. + +When translating C headers to Rust FFI, it is worth verifying for which targets +the `const` attribute is enabled in those headers, and using the appropriate +`cfg` macros in the Rust side to match those definitions. While the semantics of +`const` are implemented identically by many C and C++ compilers, e.g., clang, +[GCC], [ARM C/C++ compiler], [IBM ILE C/C++], etc. they are not necessarily +implemented in this way on all of them. It is therefore also worth verifying +that the semantics of the C toolchain used to compile the binary being linked +against are compatible with those of the `#[c_ffi_const]`. + +[ARM C/C++ compiler]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/Cacgigch.html +[GCC]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-const-function-attribute +[IBM ILE C/C++]: https://www.ibm.com/support/knowledgecenter/fr/ssw_ibm_i_71/rzarg/fn_attrib_const.htm diff --git a/src/doc/unstable-book/src/language-features/c_ffi_pure.md b/src/doc/unstable-book/src/language-features/c_ffi_pure.md new file mode 100644 index 0000000000000..b1182883c9d25 --- /dev/null +++ b/src/doc/unstable-book/src/language-features/c_ffi_pure.md @@ -0,0 +1,55 @@ +# `c_ffi_pure` + +The `#[c_ffi_pure]` attribute applies clang's `pure` attribute to foreign +functions declarations. + +That is, `#[c_ffi_pure]` functions shall have no effects except for its return +value, which shall not change across two consecutive function calls and can only +depend on the values of the function parameters and/or global memory. + +The behavior of calling a `#[c_ffi_pure]` function that violates these +requirements is undefined. + +This attribute enables Rust to perform common optimizations, like sub-expression +elimination and loop optimizations. Some common examples of pure functions are +`strlen` or `memcmp`. + +These optimizations only apply across successive invocations of the function, +since any other function could modify global memory read by `#[c_ffi_pure]` +functions, altering their result. The `#[c_ffi_const]` attribute allows +sub-expression elimination regardless of any operations in between the function +calls. + +## Pitfals + +A `#[c_ffi_pure]` function can read global memory through the function +parameters (e.g. pointers), globals, etc. `#[c_ffi_pure]` functions are not +referentially-transparent, and are therefore more relaxed than `#[c_ffi_const]` +functions. + +However, accesing global memory through volatile or atomic reads can violate the +requirement that two consecutive function calls shall return the same value. + +A `pure` function that returns unit has no effect on the abstract machine's +state. + +A diverging and `pure` C or C++ function is unreachable. Diverging via a +side-effect (e.g. a call to `abort`) violates `pure` requirements. Divergence +without side-effects is undefined behavior in C++ and not possible in C. In C++, +the behavior of infinite loops without side-effects is undefined, while in C +these loops can be assumed to terminate. This would cause a diverging function +to return, invoking undefined behavior. + +When translating C headers to Rust FFI, it is worth verifying for which targets +the `pure` attribute is enabled in those headers, and using the appropriate +`cfg` macros in the Rust side to match those definitions. While the semantics of +`pure` are implemented identically by many C and C++ compilers, e.g., clang, +[GCC], [ARM C/C++ compiler], [IBM ILE C/C++], etc. they are not necessarily +implemented in this way on all of them. It is therefore also worth verifying +that the semantics of the C toolchain used to compile the binary being linked +against are compatible with those of the `#[c_ffi_pure]`. + + +[ARM C/C++ compiler]: http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/Cacigdac.html +[GCC]: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-pure-function-attribute +[IBM ILE C/C++]: https://www.ibm.com/support/knowledgecenter/fr/ssw_ibm_i_71/rzarg/fn_attrib_pure.htm diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 8ce6394091c1a..ae227ec9eae52 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2489,29 +2489,12 @@ bitflags! { /// #[used], indicates that LLVM can't eliminate this function (but the /// linker can!) const USED = 1 << 9; - /// #[ffi_pure], indicates that a function has no effects except for - /// its return value, that its return value depends only on the - /// parameters and/or global variables, and that its return value does - /// not change between two consecutive calls (for example, because of - /// volatile access to the global variables). - /// - /// Such a function can be subject to common sub-expression elimination - /// and loop optimization just as an arithmetic operator would be. Some - /// common examples of pure functions are `strlen` or `memcmp`. - const FFI_PURE = 1 << 10; - /// #[ffi_const], indicates that a function has no effects except for - /// its return value and that its return value depends only on the - /// value of the function parameters. - /// - /// This attribute is stricter than `#[ffi_pure]`, since the function - /// return value is not allowed to depend on anything that is not the - /// value of the function parameters, like global memory, or other - /// values, like dereferencing a pointer function parameter to read - /// another value. - /// - /// A `#[ffi_const]` function that returns a unit type has no effect on the abstract machine's state. - /// abstract machine. - const FFI_CONST = 1 << 11; + /// #[c_ffi_pure]: applies clang's `pure` attribute to a foreign function + /// declaration. + const C_FFI_PURE = 1 << 10; + /// #[c_ffi_const]: applies clang's `const` attribute to a foreign function + /// declaration. + const C_FFI_CONST = 1 << 11; } } diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 8ca891a51aa7c..0dabfd8e8aef4 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -223,10 +223,10 @@ pub fn from_fn_attrs( if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::COLD) { Attribute::Cold.apply_llfn(Function, llfn); } - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::FFI_PURE) { + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::C_FFI_PURE) { Attribute::ReadOnly.apply_llfn(Function, llfn); } - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::FFI_CONST) { + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::C_FFI_CONST) { Attribute::ReadNone.apply_llfn(Function, llfn); } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index cfe409c37e2e1..bcff77db40b61 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2270,28 +2270,37 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR; } else if attr.check_name("unwind") { codegen_fn_attrs.flags |= CodegenFnAttrFlags::UNWIND; - } else if attr.check_name("ffi_pure") { + } else if attr.check_name("c_ffi_pure") { if tcx.is_foreign_item(id) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_PURE; + codegen_fn_attrs.flags |= CodegenFnAttrFlags::C_FFI_PURE; } else { - // `#[ffi_pure]` is only allowed `extern fn`s + // `#[c_ffi_pure]` is only allowed on foreign functions struct_span_err!( tcx.sess, attr.span, E0724, - "`#[ffi_pure]` may only be used on foreign functions" + "`#[c_ffi_pure]` may only be used on foreign functions" ).emit(); } - } else if attr.check_name("ffi_const") { + } else if attr.check_name("c_ffi_const") { if tcx.is_foreign_item(id) { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_CONST; + codegen_fn_attrs.flags |= CodegenFnAttrFlags::C_FFI_CONST; } else { - // `#[ffi_const]` is only allowed `extern fn`s + // `#[c_ffi_const]` is only allowed on foreign functions struct_span_err!( tcx.sess, attr.span, E0725, - "`#[ffi_const]` may only be used on foreign functions" + "`#[c_ffi_const]` may only be used on foreign functions" + ).emit(); + } + if attrs.iter().any(|a| a.check_name("c_ffi_pure")) { + // `#[c_ffi_const]` functions cannot be `#[c_ffi_pure]` + struct_span_err!( + tcx.sess, + attr.span, + E0726, + "`#[c_ffi_const]` function cannot be`#[c_ffi_pure]`" ).emit(); } } else if attr.check_name("rustc_allocator_nounwind") { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index aa8a59791a7ba..baf756ae44fcd 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -4720,6 +4720,7 @@ register_diagnostics! { E0698, // type inside generator must be known in this context E0719, // duplicate values for associated type binding E0722, // Malformed #[optimize] attribute - E0724, // `#[ffi_pure]` is only allowed on foreign fn - E0725, // `#[ffi_const]` is only allowed on foreign fn + E0724, // `#[c_ffi_pure]` is only allowed on foreign functions + E0725, // `#[c_ffi_const]` is only allowed on foreign functions + E0726, // `#[c_ffi_const]` functions cannot be `#[c_ffi_pure]` } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 39fe5fa7516e7..e97a498205950 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -290,11 +290,11 @@ declare_features! ( // The `repr(i128)` annotation for enums. (active, repr128, "1.16.0", Some(35118), None), - // Allows the use of `#[ffi_pure]` on foreign functions. - (active, ffi_pure, "1.34.0", Some(58329), None), + // Allows the use of `#[c_ffi_pure]` on foreign functions. + (active, c_ffi_pure, "1.34.0", Some(58329), None), - // Allows the use of `#[ffi_const]` on foreign functions. - (active, ffi_const, "1.34.0", Some(58328), None), + // Allows the use of `#[c_ffi_const]` on foreign functions. + (active, c_ffi_const, "1.34.0", Some(58328), None), // The `unadjusted` ABI; perma-unstable. // @@ -1130,16 +1130,16 @@ pub const BUILTIN_ATTRIBUTES: &[(&str, AttributeType, AttributeTemplate, Attribu "the `#[naked]` attribute \ is an experimental feature", cfg_fn!(naked_functions))), - ("ffi_pure", Whitelisted, template!(Word), Gated(Stability::Unstable, - "ffi_pure", - "the `#[ffi_pure]` attribute \ + ("c_ffi_pure", Whitelisted, template!(Word), Gated(Stability::Unstable, + "c_ffi_pure", + "the `#[c_ffi_pure]` attribute \ is an experimental feature", - cfg_fn!(ffi_pure))), - ("ffi_const", Whitelisted, template!(Word), Gated(Stability::Unstable, - "ffi_const", - "the `#[ffi_const]` attribute \ + cfg_fn!(c_ffi_pure))), + ("c_ffi_const", Whitelisted, template!(Word), Gated(Stability::Unstable, + "c_ffi_const", + "the `#[c_ffi_const]` attribute \ is an experimental feature", - cfg_fn!(ffi_const))), + cfg_fn!(c_ffi_const))), ("target_feature", Whitelisted, template!(List: r#"enable = "name""#), Ungated), ("export_name", Whitelisted, template!(NameValueStr: "name"), Ungated), ("inline", Whitelisted, template!(Word, List: "always|never"), Ungated), diff --git a/src/test/codegen/ffi-const.rs b/src/test/codegen/c-ffi-const.rs similarity index 81% rename from src/test/codegen/ffi-const.rs rename to src/test/codegen/c-ffi-const.rs index 440d022a12cba..1f60a965f92e9 100644 --- a/src/test/codegen/ffi-const.rs +++ b/src/test/codegen/c-ffi-const.rs @@ -1,6 +1,6 @@ // compile-flags: -C no-prepopulate-passes #![crate_type = "lib"] -#![feature(ffi_const)] +#![feature(c_ffi_const)] pub fn bar() { unsafe { foo() } } @@ -8,5 +8,5 @@ extern { // CHECK-LABEL: declare void @foo() // CHECK-SAME: [[ATTRS:#[0-9]+]] // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}readnone{{.*}} } - #[ffi_const] pub fn foo(); + #[c_ffi_const] pub fn foo(); } diff --git a/src/test/codegen/ffi-pure.rs b/src/test/codegen/c-ffi-pure.rs similarity index 82% rename from src/test/codegen/ffi-pure.rs rename to src/test/codegen/c-ffi-pure.rs index f0ebc1caa09bd..e60d88f63e33f 100644 --- a/src/test/codegen/ffi-pure.rs +++ b/src/test/codegen/c-ffi-pure.rs @@ -1,6 +1,6 @@ // compile-flags: -C no-prepopulate-passes #![crate_type = "lib"] -#![feature(ffi_pure)] +#![feature(c_ffi_pure)] pub fn bar() { unsafe { foo() } } @@ -8,5 +8,5 @@ extern { // CHECK-LABEL: declare void @foo() // CHECK-SAME: [[ATTRS:#[0-9]+]] // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}readonly{{.*}} } - #[ffi_pure] pub fn foo(); + #[c_ffi_pure] pub fn foo(); } diff --git a/src/test/ui/c_ffi_const.rs b/src/test/ui/c_ffi_const.rs new file mode 100644 index 0000000000000..2e3511655e8a0 --- /dev/null +++ b/src/test/ui/c_ffi_const.rs @@ -0,0 +1,10 @@ +// ignore-tidy-linelength +#![feature(c_ffi_const, c_ffi_pure)] +#![crate_type = "lib"] + +#[c_ffi_const] //~ ERROR `#[c_ffi_const]` may only be used on foreign functions [E0725] +pub fn foo() {} + +#[c_ffi_pure] +#[c_ffi_const] //~ ERROR `#[c_ffi_const]` functions cannot be `#[c_ffi_pure]` [E0726] +pub fn bar() {} diff --git a/src/test/ui/ffi_const.stderr b/src/test/ui/c_ffi_const.stderr similarity index 100% rename from src/test/ui/ffi_const.stderr rename to src/test/ui/c_ffi_const.stderr diff --git a/src/test/ui/c_ffi_pure.rs b/src/test/ui/c_ffi_pure.rs new file mode 100644 index 0000000000000..63a5438365fd2 --- /dev/null +++ b/src/test/ui/c_ffi_pure.rs @@ -0,0 +1,6 @@ +// ignore-tidy-linelength +#![feature(c_ffi_pure)] +#![crate_type = "lib"] + +#[c_ffi_pure] //~ ERROR `#[c_ffi_pure]` may only be used on foreign functions [E0724] +pub fn foo() {} diff --git a/src/test/ui/ffi_pure.stderr b/src/test/ui/c_ffi_pure.stderr similarity index 100% rename from src/test/ui/ffi_pure.stderr rename to src/test/ui/c_ffi_pure.stderr diff --git a/src/test/ui/feature-gates/feature-gate-c_ffi_const.rs b/src/test/ui/feature-gates/feature-gate-c_ffi_const.rs new file mode 100644 index 0000000000000..b4c23c8589489 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-c_ffi_const.rs @@ -0,0 +1,7 @@ +// ignore-tidy-linelength +#![crate_type = "lib"] + +extern { + #[c_ffi_const] //~ ERROR the `#[c_ffi_const]` attribute is an experimental feature (see issue #58328) + pub fn foo(); +} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_const.stderr b/src/test/ui/feature-gates/feature-gate-c_ffi_const.stderr similarity index 100% rename from src/test/ui/feature-gates/feature-gate-ffi_const.stderr rename to src/test/ui/feature-gates/feature-gate-c_ffi_const.stderr diff --git a/src/test/ui/feature-gates/feature-gate-c_ffi_pure.rs b/src/test/ui/feature-gates/feature-gate-c_ffi_pure.rs new file mode 100644 index 0000000000000..70f1e40443290 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-c_ffi_pure.rs @@ -0,0 +1,7 @@ +// ignore-tidy-linelength +#![crate_type = "lib"] + +extern { + #[c_ffi_pure] //~ ERROR the `#[c_ffi_pure]` attribute is an experimental feature (see issue #58329) + pub fn foo(); +} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_pure.stderr b/src/test/ui/feature-gates/feature-gate-c_ffi_pure.stderr similarity index 100% rename from src/test/ui/feature-gates/feature-gate-ffi_pure.stderr rename to src/test/ui/feature-gates/feature-gate-c_ffi_pure.stderr diff --git a/src/test/ui/feature-gates/feature-gate-ffi_const.rs b/src/test/ui/feature-gates/feature-gate-ffi_const.rs deleted file mode 100644 index 7891fea430468..0000000000000 --- a/src/test/ui/feature-gates/feature-gate-ffi_const.rs +++ /dev/null @@ -1,7 +0,0 @@ -// ignore-tidy-linelength -#![crate_type = "lib"] - -extern { - #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58328) - pub fn foo(); -} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_pure.rs b/src/test/ui/feature-gates/feature-gate-ffi_pure.rs deleted file mode 100644 index 9db77afda6c84..0000000000000 --- a/src/test/ui/feature-gates/feature-gate-ffi_pure.rs +++ /dev/null @@ -1,7 +0,0 @@ -// ignore-tidy-linelength -#![crate_type = "lib"] - -extern { - #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58329) - pub fn foo(); -} diff --git a/src/test/ui/ffi_const.rs b/src/test/ui/ffi_const.rs deleted file mode 100644 index 11554ffb3285d..0000000000000 --- a/src/test/ui/ffi_const.rs +++ /dev/null @@ -1,6 +0,0 @@ -// ignore-tidy-linelength -#![feature(ffi_const)] -#![crate_type = "lib"] - -#[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions [E0725] -pub fn foo() {} diff --git a/src/test/ui/ffi_pure.rs b/src/test/ui/ffi_pure.rs deleted file mode 100644 index 29d4142d8f25a..0000000000000 --- a/src/test/ui/ffi_pure.rs +++ /dev/null @@ -1,6 +0,0 @@ -// ignore-tidy-linelength -#![feature(ffi_pure)] -#![crate_type = "lib"] - -#[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions [E0724] -pub fn foo() {} From c11a27f87135a083fd98486f82883afc8d1f3fe5 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 12 Feb 2019 05:25:50 +0100 Subject: [PATCH 08/11] Fix tests --- src/librustc_typeck/collect.rs | 20 +++++++++---------- src/test/ui/c_ffi_const.rs | 4 ---- src/test/ui/c_ffi_const.stderr | 8 ++++---- src/test/ui/c_ffi_const2.rs | 12 +++++++++++ src/test/ui/c_ffi_const2.stderr | 9 +++++++++ src/test/ui/c_ffi_pure.stderr | 8 ++++---- .../feature-gate-c_ffi_const.stderr | 10 +++++----- .../feature-gate-c_ffi_pure.stderr | 10 +++++----- 8 files changed, 49 insertions(+), 32 deletions(-) create mode 100644 src/test/ui/c_ffi_const2.rs create mode 100644 src/test/ui/c_ffi_const2.stderr diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index bcff77db40b61..612d236805dc9 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2272,6 +2272,15 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen codegen_fn_attrs.flags |= CodegenFnAttrFlags::UNWIND; } else if attr.check_name("c_ffi_pure") { if tcx.is_foreign_item(id) { + if attrs.iter().any(|a| a.check_name("c_ffi_const")) { + // `#[c_ffi_const]` functions cannot be `#[c_ffi_pure]` + struct_span_err!( + tcx.sess, + attr.span, + E0726, + "`#[c_ffi_const]` function cannot be`#[c_ffi_pure]`" + ).emit(); + } codegen_fn_attrs.flags |= CodegenFnAttrFlags::C_FFI_PURE; } else { // `#[c_ffi_pure]` is only allowed on foreign functions @@ -2294,16 +2303,7 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen "`#[c_ffi_const]` may only be used on foreign functions" ).emit(); } - if attrs.iter().any(|a| a.check_name("c_ffi_pure")) { - // `#[c_ffi_const]` functions cannot be `#[c_ffi_pure]` - struct_span_err!( - tcx.sess, - attr.span, - E0726, - "`#[c_ffi_const]` function cannot be`#[c_ffi_pure]`" - ).emit(); - } - } else if attr.check_name("rustc_allocator_nounwind") { + } else if attr.check_name("rustc_allocator_nounwind") { codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND; } else if attr.check_name("naked") { codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED; diff --git a/src/test/ui/c_ffi_const.rs b/src/test/ui/c_ffi_const.rs index 2e3511655e8a0..72e88acc4092c 100644 --- a/src/test/ui/c_ffi_const.rs +++ b/src/test/ui/c_ffi_const.rs @@ -4,7 +4,3 @@ #[c_ffi_const] //~ ERROR `#[c_ffi_const]` may only be used on foreign functions [E0725] pub fn foo() {} - -#[c_ffi_pure] -#[c_ffi_const] //~ ERROR `#[c_ffi_const]` functions cannot be `#[c_ffi_pure]` [E0726] -pub fn bar() {} diff --git a/src/test/ui/c_ffi_const.stderr b/src/test/ui/c_ffi_const.stderr index f74c7787bab0a..9e6a3c75622ea 100644 --- a/src/test/ui/c_ffi_const.stderr +++ b/src/test/ui/c_ffi_const.stderr @@ -1,8 +1,8 @@ -error[E0725]: `#[ffi_const]` may only be used on foreign functions - --> $DIR/ffi_const.rs:5:1 +error[E0725]: `#[c_ffi_const]` may only be used on foreign functions + --> $DIR/c_ffi_const.rs:5:1 | -LL | #[ffi_const] //~ ERROR `#[ffi_const]` may only be used on foreign functions [E0725] - | ^^^^^^^^^^^^ +LL | #[c_ffi_const] //~ ERROR `#[c_ffi_const]` may only be used on foreign functions [E0725] + | ^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/c_ffi_const2.rs b/src/test/ui/c_ffi_const2.rs new file mode 100644 index 0000000000000..dc1441d7203b5 --- /dev/null +++ b/src/test/ui/c_ffi_const2.rs @@ -0,0 +1,12 @@ +// ignore-tidy-linelength +#![feature(c_ffi_const, c_ffi_pure)] + +extern { + #[c_ffi_pure] //~ ERROR `#[c_ffi_const]` function cannot be`#[c_ffi_pure]` [E0726] + #[c_ffi_const] + pub fn baz(); +} + +fn main() { + unsafe { baz() }; +} diff --git a/src/test/ui/c_ffi_const2.stderr b/src/test/ui/c_ffi_const2.stderr new file mode 100644 index 0000000000000..1fb1a2bb3a623 --- /dev/null +++ b/src/test/ui/c_ffi_const2.stderr @@ -0,0 +1,9 @@ +error[E0726]: `#[c_ffi_const]` function cannot be`#[c_ffi_pure]` + --> $DIR/c_ffi_const2.rs:5:5 + | +LL | #[c_ffi_pure] //~ ERROR `#[c_ffi_const]` function cannot be`#[c_ffi_pure]` [E0726] + | ^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0726`. diff --git a/src/test/ui/c_ffi_pure.stderr b/src/test/ui/c_ffi_pure.stderr index 11f134ce95382..c7c2c063312cb 100644 --- a/src/test/ui/c_ffi_pure.stderr +++ b/src/test/ui/c_ffi_pure.stderr @@ -1,8 +1,8 @@ -error[E0724]: `#[ffi_pure]` may only be used on foreign functions - --> $DIR/ffi_pure.rs:5:1 +error[E0724]: `#[c_ffi_pure]` may only be used on foreign functions + --> $DIR/c_ffi_pure.rs:5:1 | -LL | #[ffi_pure] //~ ERROR `#[ffi_pure]` may only be used on foreign functions [E0724] - | ^^^^^^^^^^^ +LL | #[c_ffi_pure] //~ ERROR `#[c_ffi_pure]` may only be used on foreign functions [E0724] + | ^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/feature-gates/feature-gate-c_ffi_const.stderr b/src/test/ui/feature-gates/feature-gate-c_ffi_const.stderr index 62852fc6510f7..a83c0251d4c8f 100644 --- a/src/test/ui/feature-gates/feature-gate-c_ffi_const.stderr +++ b/src/test/ui/feature-gates/feature-gate-c_ffi_const.stderr @@ -1,10 +1,10 @@ -error[E0658]: the `#[ffi_const]` attribute is an experimental feature (see issue #58328) - --> $DIR/feature-gate-ffi_const.rs:5:5 +error[E0658]: the `#[c_ffi_const]` attribute is an experimental feature (see issue #58328) + --> $DIR/feature-gate-c_ffi_const.rs:5:5 | -LL | #[ffi_const] //~ ERROR the `#[ffi_const]` attribute is an experimental feature (see issue #58328) - | ^^^^^^^^^^^^ +LL | #[c_ffi_const] //~ ERROR the `#[c_ffi_const]` attribute is an experimental feature (see issue #58328) + | ^^^^^^^^^^^^^^ | - = help: add #![feature(ffi_const)] to the crate attributes to enable + = help: add #![feature(c_ffi_const)] to the crate attributes to enable error: aborting due to previous error diff --git a/src/test/ui/feature-gates/feature-gate-c_ffi_pure.stderr b/src/test/ui/feature-gates/feature-gate-c_ffi_pure.stderr index 9108e13afb91c..1545a6e63472d 100644 --- a/src/test/ui/feature-gates/feature-gate-c_ffi_pure.stderr +++ b/src/test/ui/feature-gates/feature-gate-c_ffi_pure.stderr @@ -1,10 +1,10 @@ -error[E0658]: the `#[ffi_pure]` attribute is an experimental feature (see issue #58329) - --> $DIR/feature-gate-ffi_pure.rs:5:5 +error[E0658]: the `#[c_ffi_pure]` attribute is an experimental feature (see issue #58329) + --> $DIR/feature-gate-c_ffi_pure.rs:5:5 | -LL | #[ffi_pure] //~ ERROR the `#[ffi_pure]` attribute is an experimental feature (see issue #58329) - | ^^^^^^^^^^^ +LL | #[c_ffi_pure] //~ ERROR the `#[c_ffi_pure]` attribute is an experimental feature (see issue #58329) + | ^^^^^^^^^^^^^ | - = help: add #![feature(ffi_pure)] to the crate attributes to enable + = help: add #![feature(c_ffi_pure)] to the crate attributes to enable error: aborting due to previous error From d663711b9318e799d1b26af131aa88f9ad11c954 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 12 Feb 2019 10:55:17 +0100 Subject: [PATCH 09/11] Tidy book --- .../src/language-features/{c_ffi_const.md => c-ffi-const.md} | 0 .../src/language-features/{c_ffi_pure.md => c-ffi-pure.md} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/doc/unstable-book/src/language-features/{c_ffi_const.md => c-ffi-const.md} (100%) rename src/doc/unstable-book/src/language-features/{c_ffi_pure.md => c-ffi-pure.md} (100%) diff --git a/src/doc/unstable-book/src/language-features/c_ffi_const.md b/src/doc/unstable-book/src/language-features/c-ffi-const.md similarity index 100% rename from src/doc/unstable-book/src/language-features/c_ffi_const.md rename to src/doc/unstable-book/src/language-features/c-ffi-const.md diff --git a/src/doc/unstable-book/src/language-features/c_ffi_pure.md b/src/doc/unstable-book/src/language-features/c-ffi-pure.md similarity index 100% rename from src/doc/unstable-book/src/language-features/c_ffi_pure.md rename to src/doc/unstable-book/src/language-features/c-ffi-pure.md From a3d0f3ce4436bd820bf90774c59f6f175e61217c Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Tue, 12 Feb 2019 18:16:10 +0100 Subject: [PATCH 10/11] Fix typos --- src/doc/unstable-book/src/language-features/c-ffi-const.md | 2 +- src/doc/unstable-book/src/language-features/c-ffi-pure.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/doc/unstable-book/src/language-features/c-ffi-const.md b/src/doc/unstable-book/src/language-features/c-ffi-const.md index db6b1a5883b92..55b4e8f27f8d9 100644 --- a/src/doc/unstable-book/src/language-features/c-ffi-const.md +++ b/src/doc/unstable-book/src/language-features/c-ffi-const.md @@ -16,7 +16,7 @@ function with the same argument values regardless of other operations being performed in between these functions calls (as opposed to `#[c_ffi_pure]` functions). -## Pitfals +## Pitfalls A `#[c_ffi_const]` function can only read global memory that would not affect its return value for the whole execution of the program (e.g. immutable global diff --git a/src/doc/unstable-book/src/language-features/c-ffi-pure.md b/src/doc/unstable-book/src/language-features/c-ffi-pure.md index b1182883c9d25..c8fa118c29853 100644 --- a/src/doc/unstable-book/src/language-features/c-ffi-pure.md +++ b/src/doc/unstable-book/src/language-features/c-ffi-pure.md @@ -20,7 +20,7 @@ functions, altering their result. The `#[c_ffi_const]` attribute allows sub-expression elimination regardless of any operations in between the function calls. -## Pitfals +## Pitfalls A `#[c_ffi_pure]` function can read global memory through the function parameters (e.g. pointers), globals, etc. `#[c_ffi_pure]` functions are not From 377072539efdecbe47c101e15a26166c9749c5b9 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Wed, 13 Feb 2019 16:57:00 +0100 Subject: [PATCH 11/11] Do not emit incorrect IR on error --- src/librustc_typeck/collect.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 612d236805dc9..3cb0f9325add4 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2280,8 +2280,9 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen E0726, "`#[c_ffi_const]` function cannot be`#[c_ffi_pure]`" ).emit(); + } else { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::C_FFI_PURE; } - codegen_fn_attrs.flags |= CodegenFnAttrFlags::C_FFI_PURE; } else { // `#[c_ffi_pure]` is only allowed on foreign functions struct_span_err!(