From 7964942515bcfbc2de6d0d755f419ec551ab6ad3 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sat, 11 Sep 2021 18:00:48 +0000 Subject: [PATCH 01/10] Allow simd_bitmask to return byte arrays --- compiler/rustc_codegen_llvm/src/intrinsic.rs | 47 ++++++++++++++----- src/test/ui/simd/intrinsic/generic-bitmask.rs | 10 ++-- .../ui/simd/intrinsic/generic-bitmask.stderr | 10 ++-- src/test/ui/simd/simd-bitmask.rs | 28 +++++++++++ 4 files changed, 73 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/simd/simd-bitmask.rs diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index e63fb22829a3f..246c94968a39b 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -19,7 +19,7 @@ use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt, LayoutOf}; use rustc_middle::ty::{self, Ty}; use rustc_middle::{bug, span_bug}; use rustc_span::{sym, symbol::kw, Span, Symbol}; -use rustc_target::abi::{self, HasDataLayout, Primitive}; +use rustc_target::abi::{self, Align, HasDataLayout, Primitive}; use rustc_target::spec::{HasTargetSpec, PanicStrategy}; use std::cmp::Ordering; @@ -1056,16 +1056,13 @@ fn generic_simd_intrinsic( if name == sym::simd_bitmask { // The `fn simd_bitmask(vector) -> unsigned integer` intrinsic takes a - // vector mask and returns an unsigned integer containing the most - // significant bit (MSB) of each lane. - - // If the vector has less than 8 lanes, a u8 is returned with zeroed - // trailing bits. + // vector mask and returns the most significant bit (MSB) of each lane in the form + // of either: + // * an unsigned integer + // * an array of `u8` + // If the vector has less than 8 lanes, a u8 is returned with zeroed trailing bits. let expected_int_bits = in_len.max(8); - match ret_ty.kind() { - ty::Uint(i) if i.bit_width() == Some(expected_int_bits) => (), - _ => return_error!("bitmask `{}`, expected `u{}`", ret_ty, expected_int_bits), - } + let expected_bytes = expected_int_bits / 8 + ((expected_int_bits % 8 > 1) as u64); // Integer vector : let (i_xn, in_elem_bitwidth) = match in_elem.kind() { @@ -1095,8 +1092,34 @@ fn generic_simd_intrinsic( let i1xn = bx.trunc(i_xn_msb, bx.type_vector(bx.type_i1(), in_len)); // Bitcast to iN: let i_ = bx.bitcast(i1xn, bx.type_ix(in_len)); - // Zero-extend iN to the bitmask type: - return Ok(bx.zext(i_, bx.type_ix(expected_int_bits))); + + match ret_ty.kind() { + ty::Uint(i) if i.bit_width() == Some(expected_int_bits) => { + // Zero-extend iN to the bitmask type: + return Ok(bx.zext(i_, bx.type_ix(expected_int_bits))); + } + ty::Array(elem, len) + if matches!(elem.kind(), ty::Uint(ty::UintTy::U8)) + && len.try_eval_usize(bx.tcx, ty::ParamEnv::reveal_all()) + == Some(expected_bytes) => + { + // Zero-extend iN to the array lengh: + let ze = bx.zext(i_, bx.type_ix(expected_bytes * 8)); + + // Convert the integer to a byte array + let ptr = bx.alloca(bx.type_ix(expected_bytes * 8), Align::ONE); + bx.store(ze, ptr, Align::ONE); + let array_ty = bx.type_array(bx.type_i8(), expected_bytes); + let ptr = bx.pointercast(ptr, bx.cx.type_ptr_to(array_ty)); + return Ok(bx.load(array_ty, ptr, Align::ONE)); + } + _ => return_error!( + "cannot return `{}`, expected `u{}` or `[u8; {}]`", + ret_ty, + expected_int_bits, + expected_bytes + ), + } } fn simd_simple_float_intrinsic( diff --git a/src/test/ui/simd/intrinsic/generic-bitmask.rs b/src/test/ui/simd/intrinsic/generic-bitmask.rs index 92c4e67dfdd19..9a23dae77b96e 100644 --- a/src/test/ui/simd/intrinsic/generic-bitmask.rs +++ b/src/test/ui/simd/intrinsic/generic-bitmask.rs @@ -51,19 +51,19 @@ fn main() { let _: u64 = simd_bitmask(m64); let _: u16 = simd_bitmask(m2); - //~^ ERROR bitmask `u16`, expected `u8` + //~^ ERROR invalid monomorphization of `simd_bitmask` intrinsic let _: u16 = simd_bitmask(m8); - //~^ ERROR bitmask `u16`, expected `u8` + //~^ ERROR invalid monomorphization of `simd_bitmask` intrinsic let _: u32 = simd_bitmask(m16); - //~^ ERROR bitmask `u32`, expected `u16` + //~^ ERROR invalid monomorphization of `simd_bitmask` intrinsic let _: u64 = simd_bitmask(m32); - //~^ ERROR bitmask `u64`, expected `u32` + //~^ ERROR invalid monomorphization of `simd_bitmask` intrinsic let _: u128 = simd_bitmask(m64); - //~^ ERROR bitmask `u128`, expected `u64` + //~^ ERROR invalid monomorphization of `simd_bitmask` intrinsic } } diff --git a/src/test/ui/simd/intrinsic/generic-bitmask.stderr b/src/test/ui/simd/intrinsic/generic-bitmask.stderr index 5aaae68cafb34..0de3f8eead86d 100644 --- a/src/test/ui/simd/intrinsic/generic-bitmask.stderr +++ b/src/test/ui/simd/intrinsic/generic-bitmask.stderr @@ -1,28 +1,28 @@ -error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: bitmask `u16`, expected `u8` +error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: cannot return `u16`, expected `u8` or `[u8; 1]` --> $DIR/generic-bitmask.rs:53:22 | LL | let _: u16 = simd_bitmask(m2); | ^^^^^^^^^^^^^^^^ -error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: bitmask `u16`, expected `u8` +error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: cannot return `u16`, expected `u8` or `[u8; 1]` --> $DIR/generic-bitmask.rs:56:22 | LL | let _: u16 = simd_bitmask(m8); | ^^^^^^^^^^^^^^^^ -error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: bitmask `u32`, expected `u16` +error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: cannot return `u32`, expected `u16` or `[u8; 2]` --> $DIR/generic-bitmask.rs:59:22 | LL | let _: u32 = simd_bitmask(m16); | ^^^^^^^^^^^^^^^^^ -error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: bitmask `u64`, expected `u32` +error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: cannot return `u64`, expected `u32` or `[u8; 4]` --> $DIR/generic-bitmask.rs:62:22 | LL | let _: u64 = simd_bitmask(m32); | ^^^^^^^^^^^^^^^^^ -error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: bitmask `u128`, expected `u64` +error[E0511]: invalid monomorphization of `simd_bitmask` intrinsic: cannot return `u128`, expected `u64` or `[u8; 8]` --> $DIR/generic-bitmask.rs:65:23 | LL | let _: u128 = simd_bitmask(m64); diff --git a/src/test/ui/simd/simd-bitmask.rs b/src/test/ui/simd/simd-bitmask.rs new file mode 100644 index 0000000000000..5ee539e5d8d95 --- /dev/null +++ b/src/test/ui/simd/simd-bitmask.rs @@ -0,0 +1,28 @@ +//run-pass +#![feature(repr_simd, platform_intrinsics)] + +extern "platform-intrinsic" { + fn simd_bitmask(v: T) -> U; +} + +#[derive(Copy, Clone)] +#[repr(simd)] +struct Simd([T; N]); + +fn main() { + unsafe { + let v = Simd::([-1, 0, -1, 0]); + let i: u8 = simd_bitmask(v); + let a: [u8; 1] = simd_bitmask(v); + + assert_eq!(i, 0b0101); + assert_eq!(a, [0b0101]); + + let v = Simd::([0, 0, -1, -1, 0, 0, 0, 0, 0, 0, 0, 0, -1, 0, -1, 0]); + let i: u16 = simd_bitmask(v); + let a: [u8; 2] = simd_bitmask(v); + + assert_eq!(i, 0b0101000000001100); + assert_eq!(a, [0b1100, 0b01010000]); + } +} From 3981ca076c2a843d1cdc230691a0772eab76426e Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sun, 12 Sep 2021 02:37:23 +0000 Subject: [PATCH 02/10] Allow simd_select_bitmask to take byte arrays --- compiler/rustc_codegen_llvm/src/intrinsic.rs | 51 +++++++++++-------- src/test/ui/simd/intrinsic/generic-select.rs | 11 ++-- .../ui/simd/intrinsic/generic-select.stderr | 22 ++++---- src/test/ui/simd/simd-bitmask.rs | 23 +++++++++ 4 files changed, 70 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 246c94968a39b..2aafac7a6dc87 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -857,28 +857,39 @@ fn generic_simd_intrinsic( let arg_tys = sig.inputs(); if name == sym::simd_select_bitmask { - let in_ty = arg_tys[0]; - let m_len = match in_ty.kind() { - // Note that this `.unwrap()` crashes for isize/usize, that's sort - // of intentional as there's not currently a use case for that. - ty::Int(i) => i.bit_width().unwrap(), - ty::Uint(i) => i.bit_width().unwrap(), - _ => return_error!("`{}` is not an integral type", in_ty), - }; require_simd!(arg_tys[1], "argument"); - let (v_len, _) = arg_tys[1].simd_size_and_type(bx.tcx()); - require!( - // Allow masks for vectors with fewer than 8 elements to be - // represented with a u8 or i8. - m_len == v_len || (m_len == 8 && v_len < 8), - "mismatched lengths: mask length `{}` != other vector length `{}`", - m_len, - v_len - ); + let (len, _) = arg_tys[1].simd_size_and_type(bx.tcx()); + + let expected_int_bits = (len.max(8) - 1).next_power_of_two(); + let expected_bytes = len / 8 + ((len % 8 > 1) as u64); + + let mask_ty = arg_tys[0]; + let mask = match mask_ty.kind() { + ty::Int(i) if i.bit_width() == Some(expected_int_bits) => args[0].immediate(), + ty::Uint(i) if i.bit_width() == Some(expected_int_bits) => args[0].immediate(), + ty::Array(elem, len) + if matches!(elem.kind(), ty::Uint(ty::UintTy::U8)) + && len.try_eval_usize(bx.tcx, ty::ParamEnv::reveal_all()) + == Some(expected_bytes) => + { + let place = PlaceRef::alloca(bx, args[0].layout); + args[0].val.store(bx, place); + let int_ty = bx.type_ix(expected_bytes * 8); + let ptr = bx.pointercast(place.llval, bx.cx.type_ptr_to(int_ty)); + bx.load(int_ty, ptr, Align::ONE) + } + _ => return_error!( + "invalid bitmask `{}`, expected `u{}` or `[u8; {}]`", + mask_ty, + expected_int_bits, + expected_bytes + ), + }; + let i1 = bx.type_i1(); - let im = bx.type_ix(v_len); - let i1xn = bx.type_vector(i1, v_len); - let m_im = bx.trunc(args[0].immediate(), im); + let im = bx.type_ix(len); + let i1xn = bx.type_vector(i1, len); + let m_im = bx.trunc(mask, im); let m_i1s = bx.bitcast(m_im, i1xn); return Ok(bx.select(m_i1s, args[1].immediate(), args[2].immediate())); } diff --git a/src/test/ui/simd/intrinsic/generic-select.rs b/src/test/ui/simd/intrinsic/generic-select.rs index 7d68af49e2868..248e82ea21cfc 100644 --- a/src/test/ui/simd/intrinsic/generic-select.rs +++ b/src/test/ui/simd/intrinsic/generic-select.rs @@ -20,8 +20,7 @@ struct b8x4(pub i8, pub i8, pub i8, pub i8); #[repr(simd)] #[derive(Copy, Clone, PartialEq)] -struct b8x8(pub i8, pub i8, pub i8, pub i8, - pub i8, pub i8, pub i8, pub i8); +struct b8x8(pub i8, pub i8, pub i8, pub i8, pub i8, pub i8, pub i8, pub i8); extern "platform-intrinsic" { fn simd_select(x: T, a: U, b: U) -> U; @@ -50,15 +49,15 @@ fn main() { //~^ ERROR found non-SIMD `u32` simd_select_bitmask(0u16, x, x); - //~^ ERROR mask length `16` != other vector length `4` - // + //~^ ERROR invalid bitmask `u16`, expected `u8` or `[u8; 1]` + simd_select_bitmask(0u8, 1u32, 2u32); //~^ ERROR found non-SIMD `u32` simd_select_bitmask(0.0f32, x, x); - //~^ ERROR `f32` is not an integral type + //~^ ERROR invalid bitmask `f32`, expected `u8` or `[u8; 1]` simd_select_bitmask("x", x, x); - //~^ ERROR `&str` is not an integral type + //~^ ERROR invalid bitmask `&str`, expected `u8` or `[u8; 1]` } } diff --git a/src/test/ui/simd/intrinsic/generic-select.stderr b/src/test/ui/simd/intrinsic/generic-select.stderr index c53d581745a7c..d576f1bc77473 100644 --- a/src/test/ui/simd/intrinsic/generic-select.stderr +++ b/src/test/ui/simd/intrinsic/generic-select.stderr @@ -1,47 +1,47 @@ error[E0511]: invalid monomorphization of `simd_select` intrinsic: mismatched lengths: mask length `8` != other vector length `4` - --> $DIR/generic-select.rs:40:9 + --> $DIR/generic-select.rs:39:9 | LL | simd_select(m8, x, x); | ^^^^^^^^^^^^^^^^^^^^^ error[E0511]: invalid monomorphization of `simd_select` intrinsic: mask element type is `u32`, expected `i_` - --> $DIR/generic-select.rs:43:9 + --> $DIR/generic-select.rs:42:9 | LL | simd_select(x, x, x); | ^^^^^^^^^^^^^^^^^^^^ error[E0511]: invalid monomorphization of `simd_select` intrinsic: mask element type is `f32`, expected `i_` - --> $DIR/generic-select.rs:46:9 + --> $DIR/generic-select.rs:45:9 | LL | simd_select(z, z, z); | ^^^^^^^^^^^^^^^^^^^^ error[E0511]: invalid monomorphization of `simd_select` intrinsic: expected SIMD argument type, found non-SIMD `u32` - --> $DIR/generic-select.rs:49:9 + --> $DIR/generic-select.rs:48:9 | LL | simd_select(m4, 0u32, 1u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error[E0511]: invalid monomorphization of `simd_select_bitmask` intrinsic: mismatched lengths: mask length `16` != other vector length `4` - --> $DIR/generic-select.rs:52:9 +error[E0511]: invalid monomorphization of `simd_select_bitmask` intrinsic: invalid bitmask `u16`, expected `u8` or `[u8; 1]` + --> $DIR/generic-select.rs:51:9 | LL | simd_select_bitmask(0u16, x, x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0511]: invalid monomorphization of `simd_select_bitmask` intrinsic: expected SIMD argument type, found non-SIMD `u32` - --> $DIR/generic-select.rs:55:9 + --> $DIR/generic-select.rs:54:9 | LL | simd_select_bitmask(0u8, 1u32, 2u32); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error[E0511]: invalid monomorphization of `simd_select_bitmask` intrinsic: `f32` is not an integral type - --> $DIR/generic-select.rs:58:9 +error[E0511]: invalid monomorphization of `simd_select_bitmask` intrinsic: invalid bitmask `f32`, expected `u8` or `[u8; 1]` + --> $DIR/generic-select.rs:57:9 | LL | simd_select_bitmask(0.0f32, x, x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error[E0511]: invalid monomorphization of `simd_select_bitmask` intrinsic: `&str` is not an integral type - --> $DIR/generic-select.rs:61:9 +error[E0511]: invalid monomorphization of `simd_select_bitmask` intrinsic: invalid bitmask `&str`, expected `u8` or `[u8; 1]` + --> $DIR/generic-select.rs:60:9 | LL | simd_select_bitmask("x", x, x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/simd/simd-bitmask.rs b/src/test/ui/simd/simd-bitmask.rs index 5ee539e5d8d95..5f6ba3bd18b3c 100644 --- a/src/test/ui/simd/simd-bitmask.rs +++ b/src/test/ui/simd/simd-bitmask.rs @@ -3,6 +3,7 @@ extern "platform-intrinsic" { fn simd_bitmask(v: T) -> U; + fn simd_select_bitmask(m: T, a: U, b: U) -> U; } #[derive(Copy, Clone)] @@ -25,4 +26,26 @@ fn main() { assert_eq!(i, 0b0101000000001100); assert_eq!(a, [0b1100, 0b01010000]); } + + unsafe { + let a = Simd::([0, 1, 2, 3, 4, 5, 6, 7]); + let b = Simd::([8, 9, 10, 11, 12, 13, 14, 15]); + let e = [0, 9, 2, 11, 12, 13, 14, 15]; + + let r = simd_select_bitmask(0b0101u8, a, b); + assert_eq!(r.0, e); + + let r = simd_select_bitmask([0b0101u8], a, b); + assert_eq!(r.0, e); + + let a = Simd::([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]); + let b = Simd::([16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31]); + let e = [16, 17, 2, 3, 20, 21, 22, 23, 24, 25, 26, 27, 12, 29, 14, 31]; + + let r = simd_select_bitmask(0b0101000000001100u16, a, b); + assert_eq!(r.0, e); + + let r = simd_select_bitmask([0b1100u8, 0b01010000u8], a, b); + assert_eq!(r.0, e); + } } From 569c51d30d0437ea03c62cd75aa1fa5bee1eaab0 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Mon, 13 Sep 2021 00:45:18 +0000 Subject: [PATCH 03/10] Fix off-by-one error uncovered by std::simd tests --- compiler/rustc_codegen_llvm/src/intrinsic.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 2aafac7a6dc87..99de11bc2c493 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -861,7 +861,7 @@ fn generic_simd_intrinsic( let (len, _) = arg_tys[1].simd_size_and_type(bx.tcx()); let expected_int_bits = (len.max(8) - 1).next_power_of_two(); - let expected_bytes = len / 8 + ((len % 8 > 1) as u64); + let expected_bytes = len / 8 + ((len % 8 > 0) as u64); let mask_ty = arg_tys[0]; let mask = match mask_ty.kind() { @@ -1073,7 +1073,7 @@ fn generic_simd_intrinsic( // * an array of `u8` // If the vector has less than 8 lanes, a u8 is returned with zeroed trailing bits. let expected_int_bits = in_len.max(8); - let expected_bytes = expected_int_bits / 8 + ((expected_int_bits % 8 > 1) as u64); + let expected_bytes = expected_int_bits / 8 + ((expected_int_bits % 8 > 0) as u64); // Integer vector : let (i_xn, in_elem_bitwidth) = match in_elem.kind() { From 1e1886908fb37dcbfb8473e00e694ef672f97088 Mon Sep 17 00:00:00 2001 From: Caleb Zulawski Date: Sat, 6 Nov 2021 02:29:03 +0000 Subject: [PATCH 04/10] Disable bitmask test on big endian --- src/test/ui/simd/simd-bitmask.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/simd/simd-bitmask.rs b/src/test/ui/simd/simd-bitmask.rs index 5f6ba3bd18b3c..14ee2e741bdfd 100644 --- a/src/test/ui/simd/simd-bitmask.rs +++ b/src/test/ui/simd/simd-bitmask.rs @@ -1,4 +1,5 @@ //run-pass +//ignore-endian-big behavior of simd_select_bitmask is endian-specific #![feature(repr_simd, platform_intrinsics)] extern "platform-intrinsic" { From 9afb241af575d2550d6e58b73cb0769a73d6a1f2 Mon Sep 17 00:00:00 2001 From: est31 Date: Tue, 9 Nov 2021 23:45:17 +0100 Subject: [PATCH 05/10] Use AddAssign impl --- compiler/rustc_ast/src/util/comments.rs | 2 +- compiler/rustc_middle/src/util/common.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_ast/src/util/comments.rs b/compiler/rustc_ast/src/util/comments.rs index 542a330a03141..02792f09ace8d 100644 --- a/compiler/rustc_ast/src/util/comments.rs +++ b/compiler/rustc_ast/src/util/comments.rs @@ -169,7 +169,7 @@ pub fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec Date: Wed, 10 Nov 2021 01:54:28 +0000 Subject: [PATCH 06/10] Add comment regarding bit order --- compiler/rustc_codegen_llvm/src/intrinsic.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index 99de11bc2c493..924bb803b368f 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -1072,6 +1072,9 @@ fn generic_simd_intrinsic( // * an unsigned integer // * an array of `u8` // If the vector has less than 8 lanes, a u8 is returned with zeroed trailing bits. + // + // The bit order of the result depends on the byte endianness, LSB-first for little + // endian and MSB-first for big endian. let expected_int_bits = in_len.max(8); let expected_bytes = expected_int_bits / 8 + ((expected_int_bits % 8 > 0) as u64); From 9a987b04668641772e2d481bdacb6e00ab3d95ef Mon Sep 17 00:00:00 2001 From: inquisitivecrystal <22333129+inquisitivecrystal@users.noreply.github.com> Date: Sun, 7 Nov 2021 19:53:26 -0800 Subject: [PATCH 07/10] Add `ty::Visibility::is_public()` --- compiler/rustc_metadata/src/rmeta/decoder.rs | 2 +- compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs | 2 +- compiler/rustc_middle/src/ty/mod.rs | 4 ++++ compiler/rustc_middle/src/ty/print/pretty.rs | 2 +- compiler/rustc_privacy/src/lib.rs | 9 ++++----- compiler/rustc_resolve/src/check_unused.rs | 3 +-- compiler/rustc_resolve/src/diagnostics.rs | 4 ++-- compiler/rustc_resolve/src/imports.rs | 2 +- compiler/rustc_typeck/src/check/expr.rs | 7 ++----- compiler/rustc_typeck/src/check/method/suggest.rs | 2 +- src/librustdoc/clean/inline.rs | 4 ++-- src/librustdoc/visit_lib.rs | 6 +++--- 12 files changed, 23 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 5e90aec003e9b..b2c7818a54218 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1179,7 +1179,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { let ctor_res = Res::Def(DefKind::Ctor(CtorOf::Variant, ctor_kind), ctor_def_id); let mut vis = self.get_visibility(ctor_def_id.index); - if ctor_def_id == def_id && vis == ty::Visibility::Public { + if ctor_def_id == def_id && vis.is_public() { // For non-exhaustive variants lower the constructor visibility to // within the crate. We only need this for fictive constructors, // for other constructors correct visibilities diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 7ea004b16f23b..eeb0a77adc0ad 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -318,7 +318,7 @@ pub fn provide(providers: &mut Providers) { } let mut add_child = |bfs_queue: &mut VecDeque<_>, child: &Export, parent: DefId| { - if child.vis != ty::Visibility::Public { + if !child.vis.is_public() { return; } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 40670f1fdcaef..673733faa766f 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -333,6 +333,10 @@ impl Visibility { Visibility::Invisible => false, } } + + pub fn is_public(self) -> bool { + matches!(self, Visibility::Public) + } } /// The crate variances map is computed during typeck and contains the diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 742005e245f9d..5d9e7aaf72f8e 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -2404,7 +2404,7 @@ fn for_each_def(tcx: TyCtxt<'_>, mut collect_fn: impl for<'b> FnMut(&'b Ident, N // Iterate external crate defs but be mindful about visibility while let Some(def) = queue.pop() { for child in tcx.item_children(def).iter() { - if child.vis != ty::Visibility::Public { + if !child.vis.is_public() { continue; } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index a01efc5d85c6e..11668146f7b10 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -543,7 +543,7 @@ impl EmbargoVisitor<'tcx> { module: LocalDefId, ) { let level = Some(AccessLevel::Reachable); - if let ty::Visibility::Public = vis { + if vis.is_public() { self.update(def_id, level); } match def_kind { @@ -580,7 +580,7 @@ impl EmbargoVisitor<'tcx> { DefKind::Struct | DefKind::Union => { // While structs and unions have type privacy, their fields do not. - if let ty::Visibility::Public = vis { + if vis.is_public() { let item = self.tcx.hir().expect_item(self.tcx.hir().local_def_id_to_hir_id(def_id)); if let hir::ItemKind::Struct(ref struct_def, _) @@ -933,7 +933,7 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { let def_id = self.tcx.hir().local_def_id(id); if let Some(exports) = self.tcx.module_exports(def_id) { for export in exports.iter() { - if export.vis == ty::Visibility::Public { + if export.vis.is_public() { if let Some(def_id) = export.res.opt_def_id() { if let Some(def_id) = def_id.as_local() { self.update(def_id, Some(AccessLevel::Exported)); @@ -1918,8 +1918,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'tcx> { /// 1. It's contained within a public type /// 2. It comes from a private crate fn leaks_private_dep(&self, item_id: DefId) -> bool { - let ret = self.required_visibility == ty::Visibility::Public - && self.tcx.is_private_dep(item_id.krate); + let ret = self.required_visibility.is_public() && self.tcx.is_private_dep(item_id.krate); tracing::debug!("leaks_private_dep(item_id={:?})={}", item_id, ret); ret diff --git a/compiler/rustc_resolve/src/check_unused.rs b/compiler/rustc_resolve/src/check_unused.rs index 760b746996196..63699128e9e16 100644 --- a/compiler/rustc_resolve/src/check_unused.rs +++ b/compiler/rustc_resolve/src/check_unused.rs @@ -32,7 +32,6 @@ use rustc_ast::visit::{self, Visitor}; use rustc_ast_lowering::ResolverAstLowering; use rustc_data_structures::fx::FxHashSet; use rustc_errors::pluralize; -use rustc_middle::ty; use rustc_session::lint::builtin::{MACRO_USE_EXTERN_CRATE, UNUSED_IMPORTS}; use rustc_session::lint::BuiltinLintDiagnostics; use rustc_span::{MultiSpan, Span, DUMMY_SP}; @@ -228,7 +227,7 @@ impl Resolver<'_> { for import in self.potentially_unused_imports.iter() { match import.kind { _ if import.used.get() - || import.vis.get() == ty::Visibility::Public + || import.vis.get().is_public() || import.span.is_dummy() => { if let ImportKind::MacroUse = import.kind { diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index ff0d76e94fdf5..094a5ed7bfbfe 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -11,7 +11,7 @@ use rustc_hir::def::{self, CtorKind, CtorOf, DefKind, NonMacroAttrKind}; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::PrimTy; use rustc_middle::bug; -use rustc_middle::ty::{self, DefIdTree}; +use rustc_middle::ty::DefIdTree; use rustc_session::Session; use rustc_span::hygiene::MacroKind; use rustc_span::lev_distance::find_best_match_for_name; @@ -1308,7 +1308,7 @@ impl<'a> Resolver<'a> { ); let def_span = self.session.source_map().guess_head_span(binding.span); let mut note_span = MultiSpan::from_span(def_span); - if !first && binding.vis == ty::Visibility::Public { + if !first && binding.vis.is_public() { note_span.push_span_label(def_span, "consider importing it directly".into()); } err.span_note(note_span, &msg); diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 936ab81914a99..4262c1e9051ee 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -164,7 +164,7 @@ fn pub_use_of_private_extern_crate_hack(import: &Import<'_>, binding: &NameBindi import: Import { kind: ImportKind::ExternCrate { .. }, .. }, .. }, - ) => import.vis.get() == ty::Visibility::Public, + ) => import.vis.get().is_public(), _ => false, } } diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index a9c0b65a0981e..5c79a067e9f04 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -35,14 +35,11 @@ use rustc_hir::{ExprKind, QPath}; use rustc_infer::infer; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::InferOk; -use rustc_middle::ty; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase}; use rustc_middle::ty::error::TypeError::{FieldMisMatch, Sorts}; use rustc_middle::ty::relate::expected_found_bool; use rustc_middle::ty::subst::SubstsRef; -use rustc_middle::ty::Ty; -use rustc_middle::ty::TypeFoldable; -use rustc_middle::ty::{AdtKind, Visibility}; +use rustc_middle::ty::{self, AdtKind, Ty, TypeFoldable}; use rustc_session::parse::feature_err; use rustc_span::edition::LATEST_STABLE_EDITION; use rustc_span::hygiene::DesugaringKind; @@ -1732,7 +1729,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .filter_map(|field| { // ignore already set fields and private fields from non-local crates if skip.iter().any(|&x| x == field.ident.name) - || (!variant.def_id.is_local() && field.vis != Visibility::Public) + || (!variant.def_id.is_local() && !field.vis.is_public()) { None } else { diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index 13f475cd9e026..71cd8a43329c5 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -1410,7 +1410,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } // We only want to suggest public or local traits (#45781). - item.vis == ty::Visibility::Public || info.def_id.is_local() + item.vis.is_public() || info.def_id.is_local() }) .is_some() }) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index d670288270a40..4a8a316037961 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -435,7 +435,7 @@ crate fn build_impl( tcx.associated_items(did) .in_definition_order() .filter_map(|item| { - if associated_trait.is_some() || item.vis == ty::Visibility::Public { + if associated_trait.is_some() || item.vis.is_public() { Some(item.clean(cx)) } else { None @@ -515,7 +515,7 @@ fn build_module( // two namespaces, so the target may be listed twice. Make sure we only // visit each node at most once. for &item in cx.tcx.item_children(did).iter() { - if item.vis == ty::Visibility::Public { + if item.vis.is_public() { let res = item.res.expect_non_local(); if let Some(def_id) = res.mod_def_id() { if did == def_id || !visited.insert(def_id) { diff --git a/src/librustdoc/visit_lib.rs b/src/librustdoc/visit_lib.rs index 3e98ba08fb9ca..791f7ff437c8b 100644 --- a/src/librustdoc/visit_lib.rs +++ b/src/librustdoc/visit_lib.rs @@ -2,7 +2,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX}; use rustc_middle::middle::privacy::{AccessLevel, AccessLevels}; -use rustc_middle::ty::{TyCtxt, Visibility}; +use rustc_middle::ty::TyCtxt; use rustc_span::symbol::sym; use crate::clean::{AttributesExt, NestedAttributesExt}; @@ -59,7 +59,7 @@ impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> { for item in self.tcx.item_children(def_id).iter() { if let Some(def_id) = item.res.opt_def_id() { if self.tcx.def_key(def_id).parent.map_or(false, |d| d == def_id.index) - || item.vis == Visibility::Public + || item.vis.is_public() { self.visit_item(item.res); } @@ -70,7 +70,7 @@ impl<'a, 'tcx> LibEmbargoVisitor<'a, 'tcx> { fn visit_item(&mut self, res: Res) { let def_id = res.def_id(); let vis = self.tcx.visibility(def_id); - let inherited_item_level = if vis == Visibility::Public { self.prev_level } else { None }; + let inherited_item_level = if vis.is_public() { self.prev_level } else { None }; let item_level = self.update(def_id, inherited_item_level); From 6622376ff65d5c5f60f8a8877b223bb69e9a9552 Mon Sep 17 00:00:00 2001 From: inquisitivecrystal <22333129+inquisitivecrystal@users.noreply.github.com> Date: Sun, 7 Nov 2021 19:54:19 -0800 Subject: [PATCH 08/10] Use computed visibility in rustdoc --- src/librustdoc/clean/mod.rs | 12 ++++++++---- src/librustdoc/clean/types.rs | 4 ++-- src/librustdoc/visit_ast.rs | 13 +++++-------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 3db0ef17fd810..a44641f4488e8 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1881,7 +1881,7 @@ fn clean_extern_crate( // this is the ID of the crate itself let crate_def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX }; let attrs = cx.tcx.hir().attrs(krate.hir_id()); - let please_inline = krate.vis.node.is_pub() + let please_inline = cx.tcx.visibility(krate.def_id).is_public() && attrs.iter().any(|a| { a.has_name(sym::doc) && match a.meta_item_list() { @@ -1933,9 +1933,12 @@ fn clean_use_statement( return Vec::new(); } + let visibility = cx.tcx.visibility(import.def_id); let attrs = cx.tcx.hir().attrs(import.hir_id()); let inline_attr = attrs.lists(sym::doc).get_word_attr(sym::inline); - let pub_underscore = import.vis.node.is_pub() && name == kw::Underscore; + let pub_underscore = visibility.is_public() && name == kw::Underscore; + let current_mod = cx.tcx.parent_module_from_def_id(import.def_id); + let parent_mod = cx.tcx.parent_module_from_def_id(current_mod); if pub_underscore { if let Some(ref inline) = inline_attr { @@ -1954,8 +1957,9 @@ fn clean_use_statement( // forcefully don't inline if this is not public or if the // #[doc(no_inline)] attribute is present. // Don't inline doc(hidden) imports so they can be stripped at a later stage. - let mut denied = !(import.vis.node.is_pub() - || (cx.render_options.document_private && import.vis.node.is_pub_restricted())) + let mut denied = !(visibility.is_public() + || (cx.render_options.document_private + && visibility.is_accessible_from(parent_mod.to_def_id(), cx.tcx))) || pub_underscore || attrs.iter().any(|a| { a.has_name(sym::doc) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index fd4d620c9591e..2dba52afcd9cd 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -254,7 +254,7 @@ impl ExternalCrate { as_keyword(Res::Def(DefKind::Mod, id.def_id.to_def_id())) } hir::ItemKind::Use(path, hir::UseKind::Single) - if item.vis.node.is_pub() => + if tcx.visibility(id.def_id).is_public() => { as_keyword(path.res.expect_non_local()) .map(|(_, prim)| (id.def_id.to_def_id(), prim)) @@ -320,7 +320,7 @@ impl ExternalCrate { as_primitive(Res::Def(DefKind::Mod, id.def_id.to_def_id())) } hir::ItemKind::Use(path, hir::UseKind::Single) - if item.vis.node.is_pub() => + if tcx.visibility(id.def_id).is_public() => { as_primitive(path.res.expect_non_local()).map(|(_, prim)| { // Pretend the primitive is local. diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 5d1f934240f03..1191a94a7039b 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -10,7 +10,6 @@ use rustc_hir::CRATE_HIR_ID; use rustc_middle::middle::privacy::AccessLevel; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::{CRATE_DEF_ID, LOCAL_CRATE}; -use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Symbol}; use std::mem; @@ -72,9 +71,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } crate fn visit(mut self) -> Module<'tcx> { - let span = self.cx.tcx.def_span(CRATE_DEF_ID); let mut top_level_module = self.visit_mod_contents( - &Spanned { span, node: hir::VisibilityKind::Public }, hir::CRATE_HIR_ID, self.cx.tcx.hir().root_module(), self.cx.tcx.crate_name(LOCAL_CRATE), @@ -134,15 +131,15 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { fn visit_mod_contents( &mut self, - vis: &hir::Visibility<'_>, id: hir::HirId, m: &'tcx hir::Mod<'tcx>, name: Symbol, ) -> Module<'tcx> { let mut om = Module::new(name, id, m.inner); + let def_id = self.cx.tcx.hir().local_def_id(id).to_def_id(); // Keep track of if there were any private modules in the path. let orig_inside_public_path = self.inside_public_path; - self.inside_public_path &= vis.node.is_pub(); + self.inside_public_path &= self.cx.tcx.visibility(def_id).is_public(); for &i in m.item_ids { let item = self.cx.tcx.hir().item(i); self.visit_item(item, None, &mut om); @@ -259,7 +256,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { let name = renamed.unwrap_or(item.ident.name); let def_id = item.def_id.to_def_id(); - let is_pub = item.vis.node.is_pub() || self.cx.tcx.has_attr(def_id, sym::macro_export); + let is_pub = self.cx.tcx.visibility(def_id).is_public(); if is_pub { self.store_path(item.def_id.to_def_id()); @@ -332,7 +329,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } } hir::ItemKind::Mod(ref m) => { - om.mods.push(self.visit_mod_contents(&item.vis, item.hir_id(), m, name)); + om.mods.push(self.visit_mod_contents(item.hir_id(), m, name)); } hir::ItemKind::Fn(..) | hir::ItemKind::ExternCrate(..) @@ -368,7 +365,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { om: &mut Module<'tcx>, ) { // If inlining we only want to include public functions. - if !self.inlining || item.vis.node.is_pub() { + if !self.inlining || self.cx.tcx.visibility(item.def_id).is_public() { om.foreigns.push((item, renamed)); } } From 8d5ef320fc6c2d4436692d1558d36bd59c49394b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 Nov 2021 14:45:05 +0100 Subject: [PATCH 09/10] Remove potential useless data for search index --- src/librustdoc/html/render/cache.rs | 85 ++++++++++++++++------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index ff1bd5e7ff289..79421c128bcf8 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -258,45 +258,52 @@ crate fn get_real_types<'tcx>( ) { let is_full_generic = ty.is_full_generic(); - if is_full_generic && generics.len() == 1 { - // In this case, no need to go through an intermediate state if the generics - // contains only one element. - // - // For example: - // - // fn foo(r: Option) {} - // - // In this case, it would contain: - // - // ``` - // [{ - // name: "option", - // generics: [{ - // name: "", - // generics: [ - // name: "Display", - // generics: [] - // }] - // }] - // }] - // ``` - // - // After removing the intermediate (unnecessary) full generic, it'll become: - // - // ``` - // [{ - // name: "option", - // generics: [{ - // name: "Display", - // generics: [] - // }] - // }] - // ``` - // - // To be noted that it can work if there is ONLY ONE generic, otherwise we still - // need to keep it as is! - res.push(generics.pop().unwrap()); - return; + if is_full_generic { + if generics.is_empty() { + // This is a type parameter with no trait bounds (for example: `T` in + // `fn f(p: T)`, so not useful for the rustdoc search because we would end up + // with an empty type with an empty name. Let's just discard it. + return; + } else if generics.len() == 1 { + // In this case, no need to go through an intermediate state if the type parameter + // contains only one trait bound. + // + // For example: + // + // `fn foo(r: Option) {}` + // + // In this case, it would contain: + // + // ``` + // [{ + // name: "option", + // generics: [{ + // name: "", + // generics: [ + // name: "Display", + // generics: [] + // }] + // }] + // }] + // ``` + // + // After removing the intermediate (unnecessary) type parameter, it'll become: + // + // ``` + // [{ + // name: "option", + // generics: [{ + // name: "Display", + // generics: [] + // }] + // }] + // ``` + // + // To be noted that it can work if there is ONLY ONE trait bound, otherwise we still + // need to keep it as is! + res.push(generics.pop().unwrap()); + return; + } } let mut index_ty = get_index_type(&ty, generics); if index_ty.name.as_ref().map(|s| s.is_empty()).unwrap_or(true) { From 7b40448a6f7ab9a3e291226bab9c4b4a48e83069 Mon Sep 17 00:00:00 2001 From: Joseph Roitman Date: Wed, 10 Nov 2021 12:37:18 +0200 Subject: [PATCH 10/10] Fix collection entry API documentation. --- library/std/src/collections/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/collections/mod.rs b/library/std/src/collections/mod.rs index a19c3431989c0..8b004525b4697 100644 --- a/library/std/src/collections/mod.rs +++ b/library/std/src/collections/mod.rs @@ -268,7 +268,7 @@ //! not. Normally, this would require a `find` followed by an `insert`, //! effectively duplicating the search effort on each insertion. //! -//! When a user calls `map.entry(&key)`, the map will search for the key and +//! When a user calls `map.entry(key)`, the map will search for the key and //! then yield a variant of the `Entry` enum. //! //! If a `Vacant(entry)` is yielded, then the key *was not* found. In this case