Skip to content

Commit

Permalink
Unrolled build for rust-lang#126604
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#126604 - kadiwa4:uplift_double_negation, r=nnethercote

Uplift `clippy::double_neg` lint as `double_negations`

Warns about cases like this:
```rust
fn main() {
    let x = 1;
    let _b = --x; //~ WARN use of a double negation
}
```

The intent is to keep people from thinking that `--x` is a prefix decrement operator. `++x`, `x++` and `x--` are invalid expressions and already have a helpful diagnostic.

I didn't add a machine-applicable suggestion to the lint because it's not entirely clear what the programmer was trying to achieve with the `--x` operation. The code that triggers the lint should always be reviewed manually.

Closes rust-lang#82987
  • Loading branch information
rust-timer authored Jan 27, 2025
2 parents 633a3fe + c1dcbeb commit d3ef4cc
Show file tree
Hide file tree
Showing 20 changed files with 254 additions and 172 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ lint_builtin_deprecated_attr_link = use of deprecated attribute `{$name}`: {$rea
lint_builtin_deref_nullptr = dereferencing a null pointer
.label = this code causes undefined behavior when executed
lint_builtin_double_negations = use of a double negation
.note = the prefix `--` could be misinterpreted as a decrement operator which exists in other languages
.note_decrement = use `-= 1` if you meant to decrement the value
.add_parens_suggestion = add parentheses for clarity
lint_builtin_ellipsis_inclusive_range_patterns = `...` range patterns are deprecated
.suggestion = use `..=` for an inclusive range
Expand Down
87 changes: 66 additions & 21 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,16 @@ use rustc_trait_selection::traits::{self};
use crate::errors::BuiltinEllipsisInclusiveRangePatterns;
use crate::lints::{
BuiltinAnonymousParams, BuiltinConstNoMangle, BuiltinDeprecatedAttrLink,
BuiltinDeprecatedAttrLinkSuggestion, BuiltinDerefNullptr,
BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives,
BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures,
BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents,
BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinMutablesTransmutes,
BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed,
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, InvalidAsmLabel,
BuiltinDeprecatedAttrLinkSuggestion, BuiltinDerefNullptr, BuiltinDoubleNegations,
BuiltinDoubleNegationsAddParens, BuiltinEllipsisInclusiveRangePatternsLint,
BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote,
BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures,
BuiltinKeywordIdents, BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc,
BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasBounds,
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub,
BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment,
BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel,
};
use crate::nonstandard_style::{MethodLateContext, method_context};
use crate::{
Expand Down Expand Up @@ -90,19 +90,11 @@ declare_lint! {

declare_lint_pass!(WhileTrue => [WHILE_TRUE]);

/// Traverse through any amount of parenthesis and return the first non-parens expression.
fn pierce_parens(mut expr: &ast::Expr) -> &ast::Expr {
while let ast::ExprKind::Paren(sub) = &expr.kind {
expr = sub;
}
expr
}

impl EarlyLintPass for WhileTrue {
#[inline]
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
if let ast::ExprKind::While(cond, _, label) = &e.kind
&& let ast::ExprKind::Lit(token_lit) = pierce_parens(cond).kind
&& let ast::ExprKind::Lit(token_lit) = cond.peel_parens().kind
&& let token::Lit { kind: token::Bool, symbol: kw::True, .. } = token_lit
&& !cond.span.from_expansion()
{
Expand Down Expand Up @@ -1576,6 +1568,58 @@ impl<'tcx> LateLintPass<'tcx> for TrivialConstraints {
}
}

declare_lint! {
/// The `double_negations` lint detects expressions of the form `--x`.
///
/// ### Example
///
/// ```rust
/// fn main() {
/// let x = 1;
/// let _b = --x;
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Negating something twice is usually the same as not negating it at all.
/// However, a double negation in Rust can easily be confused with the
/// prefix decrement operator that exists in many languages derived from C.
/// Use `-(-x)` if you really wanted to negate the value twice.
///
/// To decrement a value, use `x -= 1` instead.
pub DOUBLE_NEGATIONS,
Warn,
"detects expressions of the form `--x`"
}

declare_lint_pass!(
/// Lint for expressions of the form `--x` that can be confused with C's
/// prefix decrement operator.
DoubleNegations => [DOUBLE_NEGATIONS]
);

impl EarlyLintPass for DoubleNegations {
#[inline]
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
// only lint on the innermost `--` in a chain of `-` operators,
// even if there are 3 or more negations
if let ExprKind::Unary(UnOp::Neg, ref inner) = expr.kind
&& let ExprKind::Unary(UnOp::Neg, ref inner2) = inner.kind
&& !matches!(inner2.kind, ExprKind::Unary(UnOp::Neg, _))
{
cx.emit_span_lint(DOUBLE_NEGATIONS, expr.span, BuiltinDoubleNegations {
add_parens: BuiltinDoubleNegationsAddParens {
start_span: inner.span.shrink_to_lo(),
end_span: inner.span.shrink_to_hi(),
},
});
}
}
}

declare_lint_pass!(
/// Does nothing as a lint pass, but registers some `Lint`s
/// which are used by other parts of the compiler.
Expand All @@ -1594,7 +1638,8 @@ declare_lint_pass!(
UNSTABLE_FEATURES,
UNREACHABLE_PUB,
TYPE_ALIAS_BOUNDS,
TRIVIAL_BOUNDS
TRIVIAL_BOUNDS,
DOUBLE_NEGATIONS
]
);

Expand Down Expand Up @@ -2651,7 +2696,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
}

declare_lint! {
/// The `deref_nullptr` lint detects when an null pointer is dereferenced,
/// The `deref_nullptr` lint detects when a null pointer is dereferenced,
/// which causes [undefined behavior].
///
/// ### Example
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ early_lint_methods!(
UnusedDocComment: UnusedDocComment,
Expr2024: Expr2024,
Precedence: Precedence,
DoubleNegations: DoubleNegations,
]
]
);
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,24 @@ pub(crate) struct BuiltinTrivialBounds<'a> {
pub predicate: Clause<'a>,
}

#[derive(LintDiagnostic)]
#[diag(lint_builtin_double_negations)]
#[note(lint_note)]
#[note(lint_note_decrement)]
pub(crate) struct BuiltinDoubleNegations {
#[subdiagnostic]
pub add_parens: BuiltinDoubleNegationsAddParens,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(lint_add_parens_suggestion, applicability = "maybe-incorrect")]
pub(crate) struct BuiltinDoubleNegationsAddParens {
#[suggestion_part(code = "(")]
pub start_span: Span,
#[suggestion_part(code = ")")]
pub end_span: Span,
}

#[derive(LintDiagnostic)]
pub(crate) enum BuiltinEllipsisInclusiveRangePatternsLint {
#[diag(lint_builtin_ellipsis_inclusive_range_patterns)]
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/.github/driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ unset CARGO_MANIFEST_DIR

# Run a lint and make sure it produces the expected output. It's also expected to exit with code 1
# FIXME: How to match the clippy invocation in compile-test.rs?
./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/double_neg.rs 2>double_neg.stderr && exit 1
sed -e "/= help: for/d" double_neg.stderr > normalized.stderr
diff -u normalized.stderr tests/ui/double_neg.stderr
./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/box_default.rs 2>box_default.stderr && exit 1
sed -e "/= help: for/d" box_default.stderr > normalized.stderr
diff -u normalized.stderr tests/ui/box_default.stderr

# make sure "clippy-driver --rustc --arg" and "rustc --arg" behave the same
SYSROOT=$(rustc --print sysroot)
Expand Down
9 changes: 4 additions & 5 deletions src/tools/clippy/book/src/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ You can configure lint levels on the command line by adding
`-A/W/D clippy::lint_name` like this:

```bash
cargo clippy -- -Aclippy::style -Wclippy::double_neg -Dclippy::perf
cargo clippy -- -Aclippy::style -Wclippy::box_default -Dclippy::perf
```

For [CI] all warnings can be elevated to errors which will in turn fail
Expand Down Expand Up @@ -101,11 +101,10 @@ You can configure lint levels in source code the same way you can configure
```rust,ignore
#![allow(clippy::style)]
#[warn(clippy::double_neg)]
#[warn(clippy::box_default)]
fn main() {
let x = 1;
let y = --x;
// ^^ warning: double negation
let _ = Box::<String>::new(Default::default());
// ^ warning: `Box::new(_)` of default value
}
```

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str>
});

// Find both the last lint declaration (declare_clippy_lint!) and the lint pass impl
while let Some(LintDeclSearchResult { content, .. }) = iter.find(|result| result.token == TokenKind::Ident) {
while let Some(LintDeclSearchResult { content, .. }) = iter.find(|result| result.token_kind == TokenKind::Ident) {
let mut iter = iter
.by_ref()
.filter(|t| !matches!(t.token_kind, TokenKind::Whitespace | TokenKind::LineComment { .. }));
Expand All @@ -465,7 +465,7 @@ fn setup_mod_file(path: &Path, lint: &LintData<'_>) -> io::Result<&'static str>
// matches `!{`
match_tokens!(iter, Bang OpenBrace);
if let Some(LintDeclSearchResult { range, .. }) =
iter.find(|result| result.token == TokenKind::CloseBrace)
iter.find(|result| result.token_kind == TokenKind::CloseBrace)
{
last_decl_curly_offset = Some(range.end);
}
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,6 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::misc::USED_UNDERSCORE_BINDING_INFO,
crate::misc::USED_UNDERSCORE_ITEMS_INFO,
crate::misc_early::BUILTIN_TYPE_SHADOW_INFO,
crate::misc_early::DOUBLE_NEG_INFO,
crate::misc_early::DUPLICATE_UNDERSCORE_ARGUMENT_INFO,
crate::misc_early::MIXED_CASE_HEX_LITERALS_INFO,
crate::misc_early::REDUNDANT_AT_REST_PATTERN_INFO,
Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ declare_with_version! { RENAMED(RENAMED_VERSION): &[(&str, &str)] = &[
("clippy::clone_double_ref", "suspicious_double_ref_op"),
#[clippy::version = ""]
("clippy::cmp_nan", "invalid_nan_comparisons"),
#[clippy::version = "1.86.0"]
("clippy::double_neg", "double_negations"),
#[clippy::version = ""]
("clippy::drop_bounds", "drop_bounds"),
#[clippy::version = ""]
Expand Down
18 changes: 0 additions & 18 deletions src/tools/clippy/clippy_lints/src/misc_early/double_neg.rs

This file was deleted.

22 changes: 0 additions & 22 deletions src/tools/clippy/clippy_lints/src/misc_early/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
mod builtin_type_shadow;
mod double_neg;
mod literal_suffix;
mod mixed_case_hex_literals;
mod redundant_at_rest_pattern;
Expand Down Expand Up @@ -85,25 +84,6 @@ declare_clippy_lint! {
"function arguments having names which only differ by an underscore"
}

declare_clippy_lint! {
/// ### What it does
/// Detects expressions of the form `--x`.
///
/// ### Why is this bad?
/// It can mislead C/C++ programmers to think `x` was
/// decremented.
///
/// ### Example
/// ```no_run
/// let mut x = 3;
/// --x;
/// ```
#[clippy::version = "pre 1.29.0"]
pub DOUBLE_NEG,
style,
"`--x`, which is a double negation of `x` and not a pre-decrement as in C/C++"
}

declare_clippy_lint! {
/// ### What it does
/// Warns on hexadecimal literals with mixed-case letter
Expand Down Expand Up @@ -352,7 +332,6 @@ declare_clippy_lint! {
declare_lint_pass!(MiscEarlyLints => [
UNNEEDED_FIELD_PATTERN,
DUPLICATE_UNDERSCORE_ARGUMENT,
DOUBLE_NEG,
MIXED_CASE_HEX_LITERALS,
UNSEPARATED_LITERAL_SUFFIX,
SEPARATED_LITERAL_SUFFIX,
Expand Down Expand Up @@ -415,7 +394,6 @@ impl EarlyLintPass for MiscEarlyLints {
if let ExprKind::Lit(lit) = expr.kind {
MiscEarlyLints::check_lit(cx, lit, expr.span);
}
double_neg::check(cx, expr);
}
}

Expand Down
10 changes: 0 additions & 10 deletions src/tools/clippy/tests/ui/double_neg.rs

This file was deleted.

11 changes: 0 additions & 11 deletions src/tools/clippy/tests/ui/double_neg.stderr

This file was deleted.

Loading

0 comments on commit d3ef4cc

Please sign in to comment.