Skip to content

Commit

Permalink
Unrolled build for rust-lang#116787
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#116787 - a-lafrance:span-internal-lint, r=oli-obk

Implement an internal lint encouraging use of `Span::eq_ctxt`

Adds a new Rustc internal lint that forbids use of `.ctxt() == .ctxt()` for spans, encouraging use of `.eq_ctxt()` instead (see rust-lang#49509).

Also fixed a few violations of the lint in the Rustc codebase (a fun additional way I could test my code). Edit: MIR opt folks I believe that's why you're CC'ed, just a heads up.

Two things I'm not sure about:
1. The way I chose to detect calls to `Span::ctxt`. I know adding diagnostic items to methods is generally discouraged, but after some searching and experimenting I couldn't find anything else that worked, so I went with it. That said, I'm happy to implement something different if there's a better way out there. (For what it's worth, if there is a better way, it might be worth documenting in the rustc-dev-guide, which I'm happy to take care of)
2. The error message for the lint. Ideally it would probably be good to give some context as to why the suggestion is made (e.g. `rustc::default_hash_types` tells the user that it's because of performance), but I don't have that context so I couldn't put it in the error message. Happy to iterate on the error message based on feedback during review.

r? ``@oli-obk``
  • Loading branch information
rust-timer authored Oct 17, 2023
2 parents bb74d1f + e89d4d4 commit 95df546
Show file tree
Hide file tree
Showing 25 changed files with 89 additions and 21 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.sess
.source_map()
.is_multiline(call_expr.span.with_lo(callee_expr.span.hi()))
&& call_expr.span.ctxt() == callee_expr.span.ctxt();
&& call_expr.span.eq_ctxt(callee_expr.span);
if call_is_multiline {
err.span_suggestion(
callee_expr.span.shrink_to_hi(),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,8 @@ lint_renamed_lint = lint `{$name}` has been renamed to `{$replace}`
lint_requested_level = requested on the command line with `{$level} {$lint_name}`
lint_span_use_eq_ctxt = use `.eq_ctxt()` instead of `.ctxt() == .ctxt()`
lint_supertrait_as_deref_target = `{$t}` implements `Deref` with supertrait `{$target_principal}` as target
.label = target type is set here
Expand Down
34 changes: 32 additions & 2 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
use crate::lints::{
BadOptAccessDiag, DefaultHashTypesDiag, DiagOutOfImpl, LintPassByHand, NonExistentDocKeyword,
QueryInstability, TyQualified, TykindDiag, TykindKind, UntranslatableDiag,
QueryInstability, SpanUseEqCtxtDiag, TyQualified, TykindDiag, TykindKind, UntranslatableDiag,
UntranslatableDiagnosticTrivial,
};
use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
use rustc_ast as ast;
use rustc_hir::def::Res;
use rustc_hir::{def_id::DefId, Expr, ExprKind, GenericArg, PatKind, Path, PathSegment, QPath};
use rustc_hir::{HirId, Impl, Item, ItemKind, Node, Pat, Ty, TyKind};
use rustc_hir::{BinOp, BinOpKind, HirId, Impl, Item, ItemKind, Node, Pat, Ty, TyKind};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::hygiene::{ExpnKind, MacroKind};
Expand Down Expand Up @@ -537,3 +537,33 @@ impl LateLintPass<'_> for BadOptAccess {
}
}
}

declare_tool_lint! {
pub rustc::SPAN_USE_EQ_CTXT,
Allow,
"forbid uses of `==` with `Span::ctxt`, suggest `Span::eq_ctxt` instead",
report_in_external_macro: true
}

declare_lint_pass!(SpanUseEqCtxt => [SPAN_USE_EQ_CTXT]);

impl<'tcx> LateLintPass<'tcx> for SpanUseEqCtxt {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) {
if let ExprKind::Binary(BinOp { node: BinOpKind::Eq, .. }, lhs, rhs) = expr.kind {
if is_span_ctxt_call(cx, lhs) && is_span_ctxt_call(cx, rhs) {
cx.emit_spanned_lint(SPAN_USE_EQ_CTXT, expr.span, SpanUseEqCtxtDiag);
}
}
}
}

fn is_span_ctxt_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
match &expr.kind {
ExprKind::MethodCall(..) => cx
.typeck_results()
.type_dependent_def_id(expr.hir_id)
.is_some_and(|call_did| cx.tcx.is_diagnostic_item(sym::SpanCtxt, call_did)),

_ => false,
}
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,8 @@ fn register_internals(store: &mut LintStore) {
store.register_late_mod_pass(|_| Box::new(BadOptAccess));
store.register_lints(&PassByValue::get_lints());
store.register_late_mod_pass(|_| Box::new(PassByValue));
store.register_lints(&SpanUseEqCtxt::get_lints());
store.register_late_mod_pass(|_| Box::new(SpanUseEqCtxt));
// FIXME(davidtwco): deliberately do not include `UNTRANSLATABLE_DIAGNOSTIC` and
// `DIAGNOSTIC_OUTSIDE_OF_IMPL` here because `-Wrustc::internal` is provided to every crate and
// these lints will trigger all of the time - change this once migration to diagnostic structs
Expand All @@ -548,6 +550,7 @@ fn register_internals(store: &mut LintStore) {
LintId::of(USAGE_OF_QUALIFIED_TY),
LintId::of(EXISTING_DOC_KEYWORD),
LintId::of(BAD_OPT_ACCESS),
LintId::of(SPAN_USE_EQ_CTXT),
],
);
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,10 @@ pub struct QueryInstability {
pub query: Symbol,
}

#[derive(LintDiagnostic)]
#[diag(lint_span_use_eq_ctxt)]
pub struct SpanUseEqCtxtDiag;

#[derive(LintDiagnostic)]
#[diag(lint_tykind_kind)]
pub struct TykindKind {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ impl<'a> CoverageSpansGenerator<'a> {

let Some(visible_macro) = curr.visible_macro(self.body_span) else { return };
if let Some(prev) = &self.some_prev
&& prev.expn_span.ctxt() == curr.expn_span.ctxt()
&& prev.expn_span.eq_ctxt(curr.expn_span)
{
return;
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_passes/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> {
AsyncClosure(closure_span) => {
self.sess.emit_err(BreakInsideAsyncBlock { span, closure_span, name });
}
UnlabeledBlock(block_span) if is_break && block_span.ctxt() == break_span.ctxt() => {
UnlabeledBlock(block_span) if is_break && block_span.eq_ctxt(break_span) => {
let suggestion = Some(OutsideLoopSuggestion { block_span, break_span });
self.sess.emit_err(OutsideLoop { span, name, is_break, suggestion });
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/span_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ impl Span {

/// This function is used as a fast path when decoding the full `SpanData` is not necessary.
/// It's a cut-down version of `data_untracked`.
#[cfg_attr(not(test), rustc_diagnostic_item = "SpanCtxt")]
#[inline]
pub fn ctxt(self) -> SyntaxContext {
if self.len_with_tag_or_marker != BASE_LEN_INTERNED_MARKER {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ symbols! {
SliceIndex,
SliceIter,
Some,
SpanCtxt,
String,
StructuralEq,
StructuralPartialEq,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool {

fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
if let Some(parent) = get_parent_expr(cx, e)
&& parent.span.ctxt() == e.span.ctxt()
&& parent.span.eq_ctxt(e.span)
{
match parent.kind {
ExprKind::Call(child, _) | ExprKind::MethodCall(_, child, _, _) | ExprKind::Index(child, _, _)
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ fn try_parse_contains<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Optio
},
],
_,
) if key_span.ctxt() == expr.span.ctxt() => {
) if key_span.eq_ctxt(expr.span) => {
let id = cx.typeck_results().type_dependent_def_id(expr.hir_id)?;
let expr = ContainsExpr {
negated,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
for element in array {
if_chain! {
if let ExprKind::Binary(ref op, ref lhs, _) = element.kind;
if has_unary_equivalent(op.node) && lhs.span.ctxt() == op.span.ctxt();
if has_unary_equivalent(op.node) && lhs.span.eq_ctxt(op.span);
let space_span = lhs.span.between(op.span);
if let Some(space_snippet) = snippet_opt(cx, space_span);
let lint_span = lhs.span.with_lo(lhs.span.hi());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl LateLintPass<'_> for UnderscoreTyped {
if !in_external_macro(cx.tcx.sess, local.span);
if let Some(ty) = local.ty; // Ensure that it has a type defined
if let TyKind::Infer = &ty.kind; // that type is '_'
if local.span.ctxt() == ty.span.ctxt();
if local.span.eq_ctxt(ty.span);
then {
// NOTE: Using `is_from_proc_macro` on `init` will require that it's initialized,
// this doesn't. Alternatively, `WithSearchPat` can be implemented for `Ty`
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/manual_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<'tcx> QuestionMark {
let Some(init) = local.init &&
local.els.is_none() &&
local.ty.is_none() &&
init.span.ctxt() == stmt.span.ctxt() &&
init.span.eq_ctxt(stmt.span) &&
let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init)
{
match if_let_or_match {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ fn check_arm<'tcx>(
}
},
};
if outer_pat.span.ctxt() == inner_scrutinee.span.ctxt();
if outer_pat.span.eq_ctxt(inner_scrutinee.span);
// match expression must be a local binding
// match <local> { .. }
if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee));
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/matches/manual_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ where
// it's being passed by value.
let scrutinee = peel_hir_expr_refs(scrutinee).0;
let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app);
let scrutinee_str = if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX {
let scrutinee_str = if scrutinee.span.eq_ctxt(expr.span) && scrutinee.precedence().order() < PREC_POSTFIX {
format!("({scrutinee_str})")
} else {
scrutinee_str.into()
Expand All @@ -130,7 +130,7 @@ where
if_chain! {
if !some_expr.needs_unsafe_block;
if let Some(func) = can_pass_as_func(cx, id, some_expr.expr);
if func.span.ctxt() == some_expr.expr.span.ctxt();
if func.span.eq_ctxt(some_expr.expr.span);
then {
snippet_with_applicability(cx, func.span, "..", &mut app).into_owned()
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/methods/map_unwrap_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub(super) fn check<'tcx>(
// lint, with note if neither arg is > 1 line and both map() and
// unwrap_or_else() have the same span
let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1;
let same_span = map_arg.span.ctxt() == unwrap_arg.span.ctxt();
let same_span = map_arg.span.eq_ctxt(unwrap_arg.span);
if same_span && !multiline {
let var_snippet = snippet(cx, recv.span, "..");
span_lint_and_sugg(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar(_)) = &arg.kind;
if let ExprKind::Call(called, [inner_expr]) = &inner_expr_with_q.kind;
if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, ..)) = &called.kind;
if expr.span.ctxt() == inner_expr.span.ctxt();
if expr.span.eq_ctxt(inner_expr.span);
let expr_ty = cx.typeck_results().expr_ty(expr);
let inner_ty = cx.typeck_results().expr_ty(inner_expr);
if expr_ty == inner_ty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions {
|| (path.ident.name == sym!(set_mode)
&& cx.tcx.is_diagnostic_item(sym::FsPermissions, adt.did()));
if let ExprKind::Lit(_) = param.kind;
if param.span.ctxt() == expr.span.ctxt();
if param.span.eq_ctxt(expr.span);

then {
let Some(snip) = snippet_opt(cx, param.span) else {
Expand All @@ -70,7 +70,7 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions {
if let Some(def_id) = cx.qpath_res(path, func.hir_id).opt_def_id();
if match_def_path(cx, def_id, &paths::PERMISSIONS_FROM_MODE);
if let ExprKind::Lit(_) = param.kind;
if param.span.ctxt() == expr.span.ctxt();
if param.span.eq_ctxt(expr.span);
if let Some(snip) = snippet_opt(cx, param.span);
if !snip.starts_with("0o");
then {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/redundant_async_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantAsyncBlock {
let Some(body_expr) = desugar_async_block(cx, expr) &&
let Some(expr) = desugar_await(peel_blocks(body_expr)) &&
// The await prefix must not come from a macro as its content could change in the future.
expr.span.ctxt() == body_expr.span.ctxt() &&
expr.span.eq_ctxt(body_expr.span) &&
// An async block does not have immediate side-effects from a `.await` point-of-view.
(!expr.can_have_side_effects() || desugar_async_block(cx, expr).is_some()) &&
let Some(shortened_span) = walk_span_to_context(expr.span, span.ctxt())
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/reference.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl EarlyLintPass for DerefAddrOf {
if_chain! {
if let ExprKind::Unary(UnOp::Deref, ref deref_target) = e.kind;
if let ExprKind::AddrOf(_, ref mutability, ref addrof_target) = without_parens(deref_target).kind;
if deref_target.span.ctxt() == e.span.ctxt();
if deref_target.span.eq_ctxt(e.span);
if !addrof_target.span.from_expansion();
then {
let mut applicability = Applicability::MachineApplicable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl LateLintPass<'_> for ConfusingXorAndPow {
if !in_external_macro(cx.sess(), expr.span)
&& let ExprKind::Binary(op, left, right) = &expr.kind
&& op.node == BinOpKind::BitXor
&& left.span.ctxt() == right.span.ctxt()
&& left.span.eq_ctxt(right.span)
&& let ExprKind::Lit(lit_left) = &left.kind
&& let ExprKind::Lit(lit_right) = &right.kind
&& matches!(lit_right.node, LitKind::Int(..) | LitKind::Float(..))
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ impl<'a> PanicExpn<'a> {
return None;
};
let result = match name {
"panic" if arg.span.ctxt() == expr.span.ctxt() => Self::Empty,
"panic" if arg.span.eq_ctxt(expr.span) => Self::Empty,
"panic" | "panic_str" => Self::Str(arg),
"panic_display" | "panic_cold_display" => {
let ExprKind::AddrOf(_, _, e) = &arg.kind else {
Expand Down
13 changes: 13 additions & 0 deletions tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Test the `rustc::span_use_eq_ctxt` internal lint
// compile-flags: -Z unstable-options

#![feature(rustc_private)]
#![deny(rustc::span_use_eq_ctxt)]
#![crate_type = "lib"]

extern crate rustc_span;
use rustc_span::Span;

pub fn f(s: Span, t: Span) -> bool {
s.ctxt() == t.ctxt() //~ ERROR use `.eq_ctxt()` instead of `.ctxt() == .ctxt()`
}
14 changes: 14 additions & 0 deletions tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: use `.eq_ctxt()` instead of `.ctxt() == .ctxt()`
--> $DIR/span_use_eq_ctxt.rs:12:5
|
LL | s.ctxt() == t.ctxt()
| ^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/span_use_eq_ctxt.rs:5:9
|
LL | #![deny(rustc::span_use_eq_ctxt)]
| ^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

0 comments on commit 95df546

Please sign in to comment.