Skip to content

Commit

Permalink
Auto merge of #118975 - GuillaumeGomez:rollup-0emhjx0, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
Rollup of 4 pull requests

Successful merges:

 - #113091 (Don't merge cfg and doc(cfg) attributes for re-exports)
 - #115660 (rustdoc: allow resizing the sidebar / hiding the top bar)
 - #118863 (rustc_mir_build: Enforce `rustc::potential_query_instability` lint)
 - #118909 (Some cleanup and improvement for invalid ref casting impl)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Dec 15, 2023
2 parents 96df494 + 25216b6 commit 4d1bd0d
Show file tree
Hide file tree
Showing 27 changed files with 822 additions and 148 deletions.
138 changes: 81 additions & 57 deletions compiler/rustc_lint/src/reference_casting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,59 +37,73 @@ declare_lint_pass!(InvalidReferenceCasting => [INVALID_REFERENCE_CASTING]);

impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
let Some((is_assignment, e)) = is_operation_we_care_about(cx, expr) else {
return;
};

let init = cx.expr_or_init(e);

let Some(ty_has_interior_mutability) = is_cast_from_const_to_mut(cx, init) else {
return;
};
let orig_cast = if init.span != e.span { Some(init.span) } else { None };
let ty_has_interior_mutability = ty_has_interior_mutability.then_some(());

cx.emit_spanned_lint(
INVALID_REFERENCE_CASTING,
expr.span,
if is_assignment {
InvalidReferenceCastingDiag::AssignToRef { orig_cast, ty_has_interior_mutability }
} else {
InvalidReferenceCastingDiag::BorrowAsMut { orig_cast, ty_has_interior_mutability }
},
);
if let Some((e, pat)) = borrow_or_assign(cx, expr) {
if matches!(pat, PatternKind::Borrow { mutbl: Mutability::Mut } | PatternKind::Assign) {
let init = cx.expr_or_init(e);

let Some(ty_has_interior_mutability) = is_cast_from_ref_to_mut_ptr(cx, init) else {
return;
};
let orig_cast = if init.span != e.span { Some(init.span) } else { None };
let ty_has_interior_mutability = ty_has_interior_mutability.then_some(());

cx.emit_spanned_lint(
INVALID_REFERENCE_CASTING,
expr.span,
if pat == PatternKind::Assign {
InvalidReferenceCastingDiag::AssignToRef {
orig_cast,
ty_has_interior_mutability,
}
} else {
InvalidReferenceCastingDiag::BorrowAsMut {
orig_cast,
ty_has_interior_mutability,
}
},
);
}
}
}
}

fn is_operation_we_care_about<'tcx>(
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum PatternKind {
Borrow { mutbl: Mutability },
Assign,
}

fn borrow_or_assign<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'tcx>,
) -> Option<(bool, &'tcx Expr<'tcx>)> {
fn deref_assign_or_addr_of<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<(bool, &'tcx Expr<'tcx>)> {
// &mut <expr>
let inner = if let ExprKind::AddrOf(_, Mutability::Mut, expr) = expr.kind {
expr
) -> Option<(&'tcx Expr<'tcx>, PatternKind)> {
fn deref_assign_or_addr_of<'tcx>(
expr: &'tcx Expr<'tcx>,
) -> Option<(&'tcx Expr<'tcx>, PatternKind)> {
// &(mut) <expr>
let (inner, pat) = if let ExprKind::AddrOf(_, mutbl, expr) = expr.kind {
(expr, PatternKind::Borrow { mutbl })
// <expr> = ...
} else if let ExprKind::Assign(expr, _, _) = expr.kind {
expr
(expr, PatternKind::Assign)
// <expr> += ...
} else if let ExprKind::AssignOp(_, expr, _) = expr.kind {
expr
(expr, PatternKind::Assign)
} else {
return None;
};

if let ExprKind::Unary(UnOp::Deref, e) = &inner.kind {
Some((!matches!(expr.kind, ExprKind::AddrOf(..)), e))
} else {
None
}
// *<inner>
let ExprKind::Unary(UnOp::Deref, e) = &inner.kind else {
return None;
};
Some((e, pat))
}

fn ptr_write<'tcx>(
cx: &LateContext<'tcx>,
e: &'tcx Expr<'tcx>,
) -> Option<(bool, &'tcx Expr<'tcx>)> {
) -> Option<(&'tcx Expr<'tcx>, PatternKind)> {
if let ExprKind::Call(path, [arg_ptr, _arg_val]) = e.kind
&& let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
Expand All @@ -98,7 +112,7 @@ fn is_operation_we_care_about<'tcx>(
Some(sym::ptr_write | sym::ptr_write_volatile | sym::ptr_write_unaligned)
)
{
Some((true, arg_ptr))
Some((arg_ptr, PatternKind::Assign))
} else {
None
}
Expand All @@ -107,20 +121,39 @@ fn is_operation_we_care_about<'tcx>(
deref_assign_or_addr_of(e).or_else(|| ptr_write(cx, e))
}

fn is_cast_from_const_to_mut<'tcx>(
fn is_cast_from_ref_to_mut_ptr<'tcx>(
cx: &LateContext<'tcx>,
orig_expr: &'tcx Expr<'tcx>,
) -> Option<bool> {
let mut need_check_freeze = false;
let mut e = orig_expr;

let end_ty = cx.typeck_results().node_type(orig_expr.hir_id);

// Bail out early if the end type is **not** a mutable pointer.
if !matches!(end_ty.kind(), ty::RawPtr(TypeAndMut { ty: _, mutbl: Mutability::Mut })) {
return None;
}

let (e, need_check_freeze) = peel_casts(cx, orig_expr);

let start_ty = cx.typeck_results().node_type(e.hir_id);
if let ty::Ref(_, inner_ty, Mutability::Not) = start_ty.kind() {
// If an UnsafeCell method is involved, we need to additionally check the
// inner type for the presence of the Freeze trait (ie does NOT contain
// an UnsafeCell), since in that case we would incorrectly lint on valid casts.
//
// Except on the presence of non concrete skeleton types (ie generics)
// since there is no way to make it safe for arbitrary types.
let inner_ty_has_interior_mutability =
!inner_ty.is_freeze(cx.tcx, cx.param_env) && inner_ty.has_concrete_skeleton();
(!need_check_freeze || !inner_ty_has_interior_mutability)
.then_some(inner_ty_has_interior_mutability)
} else {
None
}
}

fn peel_casts<'tcx>(cx: &LateContext<'tcx>, mut e: &'tcx Expr<'tcx>) -> (&'tcx Expr<'tcx>, bool) {
let mut gone_trough_unsafe_cell_raw_get = false;

loop {
e = e.peel_blocks();
// <expr> as ...
Expand All @@ -145,27 +178,18 @@ fn is_cast_from_const_to_mut<'tcx>(
)
{
if cx.tcx.is_diagnostic_item(sym::unsafe_cell_raw_get, def_id) {
need_check_freeze = true;
gone_trough_unsafe_cell_raw_get = true;
}
arg
} else {
break;
let init = cx.expr_or_init(e);
if init.hir_id != e.hir_id {
init
} else {
break;
}
};
}

let start_ty = cx.typeck_results().node_type(e.hir_id);
if let ty::Ref(_, inner_ty, Mutability::Not) = start_ty.kind() {
// If an UnsafeCell method is involved we need to additionally check the
// inner type for the presence of the Freeze trait (ie does NOT contain
// an UnsafeCell), since in that case we would incorrectly lint on valid casts.
//
// We also consider non concrete skeleton types (ie generics)
// to be an issue since there is no way to make it safe for abitrary types.
let inner_ty_has_interior_mutability =
!inner_ty.is_freeze(cx.tcx, cx.param_env) && inner_ty.has_concrete_skeleton();
(!need_check_freeze || !inner_ty_has_interior_mutability)
.then_some(inner_ty_has_interior_mutability)
} else {
None
}
(e, gone_trough_unsafe_cell_raw_get)
}
1 change: 0 additions & 1 deletion compiler/rustc_mir_build/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Construction of MIR from HIR.
//!
//! This crate also contains the match exhaustiveness and usefulness checking.
#![allow(rustc::potential_query_instability)]
#![feature(assert_matches)]
#![feature(associated_type_bounds)]
#![feature(box_patterns)]
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::errors::*;

use rustc_arena::TypedArena;
use rustc_ast::Mutability;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::fx::FxIndexSet;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorGuaranteed, MultiSpan};
use rustc_hir as hir;
Expand Down Expand Up @@ -948,7 +948,7 @@ fn report_non_exhaustive_match<'p, 'tcx>(
err.note(format!("the matched value is of type `{}`", scrut_ty));

if !is_empty_match {
let mut non_exhaustive_tys = FxHashSet::default();
let mut non_exhaustive_tys = FxIndexSet::default();
// Look at the first witness.
collect_non_exhaustive_tys(cx, &witnesses[0], &mut non_exhaustive_tys);

Expand Down Expand Up @@ -1104,7 +1104,7 @@ fn joined_uncovered_patterns<'p, 'tcx>(
fn collect_non_exhaustive_tys<'tcx>(
cx: &MatchCheckCtxt<'_, 'tcx>,
pat: &WitnessPat<'tcx>,
non_exhaustive_tys: &mut FxHashSet<Ty<'tcx>>,
non_exhaustive_tys: &mut FxIndexSet<Ty<'tcx>>,
) {
if matches!(pat.ctor(), Constructor::NonExhaustive) {
non_exhaustive_tys.insert(pat.ty());
Expand Down
76 changes: 42 additions & 34 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2646,6 +2646,40 @@ fn filter_tokens_from_list(
tokens
}

fn filter_doc_attr_ident(ident: Symbol, is_inline: bool) -> bool {
if is_inline {
ident == sym::hidden || ident == sym::inline || ident == sym::no_inline
} else {
ident == sym::cfg
}
}

/// Remove attributes from `normal` that should not be inherited by `use` re-export.
/// Before calling this function, make sure `normal` is a `#[doc]` attribute.
fn filter_doc_attr(normal: &mut ast::NormalAttr, is_inline: bool) {
match normal.item.args {
ast::AttrArgs::Delimited(ref mut args) => {
let tokens = filter_tokens_from_list(&args.tokens, |token| {
!matches!(
token,
TokenTree::Token(
Token {
kind: TokenKind::Ident(
ident,
_,
),
..
},
_,
) if filter_doc_attr_ident(*ident, is_inline),
)
});
args.tokens = TokenStream::new(tokens);
}
ast::AttrArgs::Empty | ast::AttrArgs::Eq(..) => {}
}
}

/// When inlining items, we merge their attributes (and all the reexports attributes too) with the
/// final reexport. For example:
///
Expand All @@ -2672,13 +2706,6 @@ fn add_without_unwanted_attributes<'hir>(
is_inline: bool,
import_parent: Option<DefId>,
) {
// If it's not `#[doc(inline)]`, we don't want all attributes, otherwise we keep everything.
if !is_inline {
for attr in new_attrs {
attrs.push((Cow::Borrowed(attr), import_parent));
}
return;
}
for attr in new_attrs {
if matches!(attr.kind, ast::AttrKind::DocComment(..)) {
attrs.push((Cow::Borrowed(attr), import_parent));
Expand All @@ -2687,33 +2714,14 @@ fn add_without_unwanted_attributes<'hir>(
let mut attr = attr.clone();
match attr.kind {
ast::AttrKind::Normal(ref mut normal) => {
if let [ident] = &*normal.item.path.segments
&& let ident = ident.ident.name
&& ident == sym::doc
{
match normal.item.args {
ast::AttrArgs::Delimited(ref mut args) => {
let tokens = filter_tokens_from_list(&args.tokens, |token| {
!matches!(
token,
TokenTree::Token(
Token {
kind: TokenKind::Ident(
sym::hidden | sym::inline | sym::no_inline,
_,
),
..
},
_,
),
)
});
args.tokens = TokenStream::new(tokens);
attrs.push((Cow::Owned(attr), import_parent));
}
ast::AttrArgs::Empty | ast::AttrArgs::Eq(..) => {
attrs.push((Cow::Owned(attr), import_parent));
}
if let [ident] = &*normal.item.path.segments {
let ident = ident.ident.name;
if ident == sym::doc {
filter_doc_attr(normal, is_inline);
attrs.push((Cow::Owned(attr), import_parent));
} else if ident != sym::cfg {
// If it's not a `cfg()` attribute, we keep it.
attrs.push((Cow::Owned(attr), import_parent));
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2013,6 +2013,7 @@ fn init_id_map() -> FxHashMap<Cow<'static, str>, usize> {
map.insert("themeStyle".into(), 1);
map.insert("settings-menu".into(), 1);
map.insert("help-button".into(), 1);
map.insert("sidebar-button".into(), 1);
map.insert("main-content".into(), 1);
map.insert("toggle-all-docs".into(), 1);
map.insert("all-types".into(), 1);
Expand Down
13 changes: 2 additions & 11 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use clean::AttributesExt;

use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir as hir;
Expand Down Expand Up @@ -465,16 +463,9 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:

clean::ImportItem(ref import) => {
let stab_tags = if let Some(import_def_id) = import.source.did {
let ast_attrs = tcx.get_attrs_unchecked(import_def_id);
let import_attrs = Box::new(clean::Attributes::from_ast(ast_attrs));

// Just need an item with the correct def_id and attrs
let import_item = clean::Item {
item_id: import_def_id.into(),
attrs: import_attrs,
cfg: ast_attrs.cfg(tcx, &cx.cache().hidden_cfg),
..myitem.clone()
};
let import_item =
clean::Item { item_id: import_def_id.into(), ..myitem.clone() };

let stab_tags = Some(extra_info_tags(&import_item, item, tcx).to_string());
stab_tags
Expand Down
6 changes: 5 additions & 1 deletion src/librustdoc/html/static/css/noscript.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ rules.
margin-left: 0 !important;
}

#copy-path {
#copy-path, #sidebar-button, .sidebar-resizer {
/* It requires JS to work so no need to display it in this case. */
display: none;
}
Expand Down Expand Up @@ -132,6 +132,8 @@ nav.sub {
--scrape-example-help-hover-color: #000;
--scrape-example-code-wrapper-background-start: rgba(255, 255, 255, 1);
--scrape-example-code-wrapper-background-end: rgba(255, 255, 255, 0);
--sidebar-resizer-hover: hsl(207, 90%, 66%);
--sidebar-resizer-active: hsl(207, 90%, 54%);
}
/* End theme: light */

Expand Down Expand Up @@ -238,6 +240,8 @@ nav.sub {
--scrape-example-help-hover-color: #fff;
--scrape-example-code-wrapper-background-start: rgba(53, 53, 53, 1);
--scrape-example-code-wrapper-background-end: rgba(53, 53, 53, 0);
--sidebar-resizer-hover: hsl(207, 30%, 54%);
--sidebar-resizer-active: hsl(207, 90%, 54%);
}
/* End theme: dark */
}
Loading

0 comments on commit 4d1bd0d

Please sign in to comment.