dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Auto merge of #125057 - nnethercote:rustc_lint-cleanups, r=cjgillot 

`rustc_lint` cleanups

Minor improvements I found while looking through this code.

r? `@cjgillot`
This commit is contained in:
bors 2024-06-03 00:04:14 +00:00
parent 032af18af5 32c8a12854
commit 9f2d0b3490
11 changed files with 117 additions and 106 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
compiler/rustc_lint/src/builtin.rs
View File

@ -1494,8 +1494,9 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
let ty = cx.tcx.type_of(item.owner_id).skip_binder();
if ty.has_inherent_projections() {
// Bounds of type aliases that contain opaque types or inherent projections are respected.
// E.g: `type X = impl Trait;`, `type X = (impl Trait, Y);`, `type X = Type::Inherent;`.
// Bounds of type aliases that contain opaque types or inherent projections are
// respected. E.g: `type X = impl Trait;`, `type X = (impl Trait, Y);`, `type X =
// Type::Inherent;`.
return;
}
@ -2224,7 +2225,8 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
hir_generics.span.shrink_to_hi().to(where_span)
};
// Due to macro expansions, the `full_where_span` might not actually contain all predicates.
// Due to macro expansions, the `full_where_span` might not actually contain all
// predicates.
if where_lint_spans.iter().all(|&sp| full_where_span.contains(sp)) {
lint_spans.push(full_where_span);
} else {
@ -2601,7 +2603,8 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
};
// So we have at least one potentially inhabited variant. Might we have two?
let Some(second_variant) = potential_variants.next() else {
// There is only one potentially inhabited variant. So we can recursively check that variant!
// There is only one potentially inhabited variant. So we can recursively
// check that variant!
return variant_find_init_error(
cx,
ty,
@ -2611,10 +2614,10 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
init,
);
};
// So we have at least two potentially inhabited variants.
// If we can prove that we have at least two *definitely* inhabited variants,
// then we have a tag and hence leaving this uninit is definitely disallowed.
// (Leaving it zeroed could be okay, depending on which variant is encoded as zero tag.)
// So we have at least two potentially inhabited variants. If we can prove that
// we have at least two *definitely* inhabited variants, then we have a tag and
// hence leaving this uninit is definitely disallowed. (Leaving it zeroed could
// be okay, depending on which variant is encoded as zero tag.)
if init == InitKind::Uninit {
let definitely_inhabited = (first_variant.1 as usize)
+ (second_variant.1 as usize)
@ -2825,7 +2828,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
let mut found_labels = Vec::new();
// A semicolon might not actually be specified as a separator for all targets, but it seems like LLVM accepts it always
// A semicolon might not actually be specified as a separator for all targets, but
// it seems like LLVM accepts it always.
let statements = template_str.split(|c| matches!(c, '\n' | ';'));
for statement in statements {
// If there's a comment, trim it from the statement
@ -2838,7 +2842,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
let mut chars = possible_label.chars();
let Some(start) = chars.next() else {
// Empty string means a leading ':' in this section, which is not a label.
// Empty string means a leading ':' in this section, which is not a
// label.
break 'label_loop;
};
@ -2855,12 +2860,15 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
// Labels continue with ASCII alphanumeric characters, _, or $
for c in chars {
// Inside a template format arg, any character is permitted for the puproses of label detection
// because we assume that it can be replaced with some other valid label string later.
// `options(raw)` asm blocks cannot have format args, so they are excluded from this special case.
// Inside a template format arg, any character is permitted for the
// puproses of label detection because we assume that it can be
// replaced with some other valid label string later. `options(raw)`
// asm blocks cannot have format args, so they are excluded from this
// special case.
if !raw && in_bracket {
if c == '{' {
// Nested brackets are not allowed in format args, this cannot be a label.
// Nested brackets are not allowed in format args, this cannot
// be a label.
break 'label_loop;
}
@ -2873,7 +2881,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
in_bracket = true;
} else {
if !(c.is_ascii_alphanumeric() || matches!(c, '_' | '$')) {
// The potential label had an invalid character inside it, it cannot be a label.
// The potential label had an invalid character inside it, it
// cannot be a label.
break 'label_loop;
}
}
@ -2892,7 +2901,8 @@ impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
.into_iter()
.filter_map(|label| find_label_span(label))
.collect::<Vec<Span>>();
// If there were labels but we couldn't find a span, combine the warnings and use the template span
// If there were labels but we couldn't find a span, combine the warnings and
// use the template span.
let target_spans: MultiSpan =
if spans.len() > 0 { spans.into() } else { (*template_span).into() };

42
compiler/rustc_lint/src/context.rs
View File

@ -94,7 +94,8 @@ enum TargetLint {
/// A lint name that should give no warnings and have no effect.
///
/// This is used by rustc to avoid warning about old rustdoc lints before rustdoc registers them as tool lints.
/// This is used by rustc to avoid warning about old rustdoc lints before rustdoc registers
/// them as tool lints.
Ignored,
}
@ -126,12 +127,16 @@ pub enum CheckLintNameResult<'a> {
Renamed(String),
/// The lint has been removed due to the given reason.
Removed(String),
/// The lint is from a tool. If the Option is None, then either
/// the lint does not exist in the tool or the code was not
/// compiled with the tool and therefore the lint was never
/// added to the `LintStore`. Otherwise the `LintId` will be
/// returned as if it where a rustc lint.
Tool(Result<&'a [LintId], (Option<&'a [LintId]>, String)>),
/// The lint is from a tool. The `LintId` will be returned as if it were a
/// rustc lint. The `Option<String>` indicates if the lint has been
/// renamed.
Tool(&'a [LintId], Option<String>),
/// The lint is from a tool. Either the lint does not exist in the tool or
/// the code was not compiled with the tool and therefore the lint was
/// never added to the `LintStore`.
MissingTool,
}
impl LintStore {
@ -384,14 +389,14 @@ impl LintStore {
} else {
// 2. The tool isn't currently running, so no lints will be registered.
// To avoid giving a false positive, ignore all unknown lints.
CheckLintNameResult::Tool(Err((None, String::new())))
CheckLintNameResult::MissingTool
};
}
Some(LintGroup { lint_ids, .. }) => {
return CheckLintNameResult::Tool(Ok(lint_ids));
return CheckLintNameResult::Tool(lint_ids, None);
}
},
Some(Id(id)) => return CheckLintNameResult::Tool(Ok(slice::from_ref(id))),
Some(Id(id)) => return CheckLintNameResult::Tool(slice::from_ref(id), None),
// If the lint was registered as removed or renamed by the lint tool, we don't need
// to treat tool_lints and rustc lints different and can use the code below.
_ => {}
@ -411,7 +416,7 @@ impl LintStore {
return if *silent {
CheckLintNameResult::Ok(lint_ids)
} else {
CheckLintNameResult::Tool(Err((Some(lint_ids), (*name).to_string())))
CheckLintNameResult::Tool(lint_ids, Some((*name).to_string()))
};
}
CheckLintNameResult::Ok(lint_ids)
@ -472,18 +477,17 @@ impl LintStore {
// Reaching this would be weird, but let's cover this case anyway
if let Some(LintAlias { name, silent }) = depr {
let LintGroup { lint_ids, .. } = self.lint_groups.get(name).unwrap();
return if *silent {
CheckLintNameResult::Tool(Err((Some(lint_ids), complete_name)))
if *silent {
CheckLintNameResult::Tool(lint_ids, Some(complete_name))
} else {
CheckLintNameResult::Tool(Err((Some(lint_ids), (*name).to_string())))
};
CheckLintNameResult::Tool(lint_ids, Some((*name).to_string()))
}
} else {
CheckLintNameResult::Tool(lint_ids, Some(complete_name))
}
CheckLintNameResult::Tool(Err((Some(lint_ids), complete_name)))
}
},
Some(Id(id)) => {
CheckLintNameResult::Tool(Err((Some(slice::from_ref(id)), complete_name)))
}
Some(Id(id)) => CheckLintNameResult::Tool(slice::from_ref(id), Some(complete_name)),
Some(other) => {
debug!("got renamed lint {:?}", other);
CheckLintNameResult::NoLint(None)

2
compiler/rustc_lint/src/errors.rs
View File

@ -16,7 +16,7 @@ pub struct OverruledAttribute<'a> {
#[subdiagnostic]
pub sub: OverruledAttributeSub,
}
//
pub enum OverruledAttributeSub {
DefaultSource { id: String },
NodeSource { span: Span, reason: Option<Symbol> },

14
compiler/rustc_lint/src/internal.rs
View File

@ -18,11 +18,11 @@ use rustc_span::Span;
use tracing::debug;
declare_tool_lint! {
/// The `default_hash_type` lint detects use of [`std::collections::HashMap`]/[`std::collections::HashSet`],
/// suggesting the use of `FxHashMap`/`FxHashSet`.
/// The `default_hash_type` lint detects use of [`std::collections::HashMap`] and
/// [`std::collections::HashSet`], suggesting the use of `FxHashMap`/`FxHashSet`.
///
/// This can help as `FxHasher` can perform better than the default hasher. DOS protection is not
/// required as input is assumed to be trusted.
/// This can help as `FxHasher` can perform better than the default hasher. DOS protection is
/// not required as input is assumed to be trusted.
pub rustc::DEFAULT_HASH_TYPES,
Allow,
"forbid HashMap and HashSet and suggest the FxHash* variants",
@ -35,7 +35,7 @@ impl LateLintPass<'_> for DefaultHashTypes {
fn check_path(&mut self, cx: &LateContext<'_>, path: &Path<'_>, hir_id: HirId) {
let Res::Def(rustc_hir::def::DefKind::Struct, def_id) = path.res else { return };
if matches!(cx.tcx.hir_node(hir_id), Node::Item(Item { kind: ItemKind::Use(..), .. })) {
// don't lint imports, only actual usages
// Don't lint imports, only actual usages.
return;
}
let preferred = match cx.tcx.get_diagnostic_name(def_id) {
@ -75,8 +75,8 @@ declare_tool_lint! {
/// potential query instability, such as iterating over a `HashMap`.
///
/// Due to the [incremental compilation](https://rustc-dev-guide.rust-lang.org/queries/incremental-compilation.html) model,
/// queries must return deterministic, stable results. `HashMap` iteration order can change between compilations,
/// and will introduce instability if query results expose the order.
/// queries must return deterministic, stable results. `HashMap` iteration order can change
/// between compilations, and will introduce instability if query results expose the order.
pub rustc::POTENTIAL_QUERY_INSTABILITY,
Allow,
"require explicit opt-in when using potentially unstable methods or functions",

10
compiler/rustc_lint/src/let_underscore.rs
View File

@ -113,11 +113,11 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
let mut top_level = true;
// We recursively walk through all patterns, so that we can catch cases where the lock is nested in a pattern.
// For the basic `let_underscore_drop` lint, we only look at the top level, since there are many legitimate reasons
// to bind a sub-pattern to an `_`, if we're only interested in the rest.
// But with locks, we prefer having the chance of "false positives" over missing cases, since the effects can be
// quite catastrophic.
// We recursively walk through all patterns, so that we can catch cases where the lock is
// nested in a pattern. For the basic `let_underscore_drop` lint, we only look at the top
// level, since there are many legitimate reasons to bind a sub-pattern to an `_`, if we're
// only interested in the rest. But with locks, we prefer having the chance of "false
// positives" over missing cases, since the effects can be quite catastrophic.
local.pat.walk_always(|pat| {
let is_top_level = top_level;
top_level = false;

82
compiler/rustc_lint/src/levels.rs
View File

@ -593,7 +593,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
let lint = UnknownLintFromCommandLine { name, suggestion, requested_level };
self.emit_lint(UNKNOWN_LINTS, lint);
}
CheckLintNameResult::Tool(Err((Some(_), ref replace))) => {
CheckLintNameResult::Tool(_, Some(ref replace)) => {
let name = lint_name.clone();
let requested_level = RequestedLevel { level, lint_name };
let lint = DeprecatedLintNameFromCommandLine { name, replace, requested_level };
@ -750,7 +750,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
let level = match Level::from_attr(attr) {
None => continue,
// This is the only lint level with a `LintExpectationId` that can be created from an attribute
// This is the only lint level with a `LintExpectationId` that can be created from
// an attribute.
Some(Level::Expect(unstable_id)) if let Some(hir_id) = source_hir_id => {
let LintExpectationId::Unstable { attr_id, lint_index } = unstable_id else {
bug!("stable id Level::from_attr")
@ -760,8 +761,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
hir_id,
attr_index: attr_index.try_into().unwrap(),
lint_index,
// we pass the previous unstable attr_id such that we can trace the ast id when building a map
// to go from unstable to stable id.
// We pass the previous unstable attr_id such that we can trace the ast id
// when building a map to go from unstable to stable id.
attr_id: Some(attr_id),
};
@ -860,13 +861,15 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
self.store.check_lint_name(&name, tool_name, self.registered_tools);
match &lint_result {
CheckLintNameResult::Ok(ids) => {
// This checks for instances where the user writes `#[expect(unfulfilled_lint_expectations)]`
// in that case we want to avoid overriding the lint level but instead add an expectation that
// can't be fulfilled. The lint message will include an explanation, that the
// This checks for instances where the user writes
// `#[expect(unfulfilled_lint_expectations)]` in that case we want to avoid
// overriding the lint level but instead add an expectation that can't be
// fulfilled. The lint message will include an explanation, that the
// `unfulfilled_lint_expectations` lint can't be expected.
if let Level::Expect(expect_id) = level {
// The `unfulfilled_lint_expectations` lint is not part of any lint groups. Therefore. we
// only need to check the slice if it contains a single lint.
// The `unfulfilled_lint_expectations` lint is not part of any lint
// groups. Therefore. we only need to check the slice if it contains a
// single lint.
let is_unfulfilled_lint_expectations = match ids {
[lint] => *lint == LintId::of(UNFULFILLED_LINT_EXPECTATIONS),
_ => false,
@ -899,32 +902,20 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
}
}
CheckLintNameResult::Tool(result) => {
match *result {
Ok(ids) => {
CheckLintNameResult::Tool(ids, new_lint_name) => {
let src = match new_lint_name {
None => {
let complete_name =
&format!("{}::{}", tool_ident.unwrap().name, name);
let src = LintLevelSource::Node {
LintLevelSource::Node {
name: Symbol::intern(complete_name),
span: sp,
reason,
};
for &id in ids {
if self.check_gated_lint(id, attr.span, false) {
self.insert_spec(id, (level, src));
}
}
if let Level::Expect(expect_id) = level {
self.provider.push_expectation(
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
);
}
}
Err((Some(ids), ref new_lint_name)) => {
let lint = builtin::RENAMED_AND_REMOVED_LINTS;
Some(new_lint_name) => {
self.emit_span_lint(
lint,
builtin::RENAMED_AND_REMOVED_LINTS,
sp.into(),
DeprecatedLintName {
name,
@ -932,29 +923,31 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
replace: new_lint_name,
},
);
let src = LintLevelSource::Node {
LintLevelSource::Node {
name: Symbol::intern(new_lint_name),
span: sp,
reason,
};
for id in ids {
self.insert_spec(*id, (level, src));
}
if let Level::Expect(expect_id) = level {
self.provider.push_expectation(
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
);
}
}
Err((None, _)) => {
// If Tool(Err(None, _)) is returned, then either the lint does not
// exist in the tool or the code was not compiled with the tool and
// therefore the lint was never added to the `LintStore`. To detect
// this is the responsibility of the lint tool.
};
for &id in *ids {
if self.check_gated_lint(id, attr.span, false) {
self.insert_spec(id, (level, src));
}
}
if let Level::Expect(expect_id) = level {
self.provider.push_expectation(
expect_id,
LintExpectation::new(reason, sp, false, tool_name),
);
}
}
CheckLintNameResult::MissingTool => {
// If `MissingTool` is returned, then either the lint does not
// exist in the tool or the code was not compiled with the tool and
// therefore the lint was never added to the `LintStore`. To detect
// this is the responsibility of the lint tool.
}
&CheckLintNameResult::NoTool => {
@ -997,7 +990,8 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
// we don't warn about the name change.
if let CheckLintNameResult::Renamed(new_name) = lint_result {
// Ignore any errors or warnings that happen because the new name is inaccurate
// NOTE: `new_name` already includes the tool name, so we don't have to add it again.
// NOTE: `new_name` already includes the tool name, so we don't have to add it
// again.
let CheckLintNameResult::Ok(ids) =
self.store.check_lint_name(&new_name, None, self.registered_tools)
else {

6
compiler/rustc_lint/src/methods.rs
View File

@ -25,9 +25,9 @@ declare_lint! {
///
/// The inner pointer of a `CString` lives only as long as the `CString` it
/// points to. Getting the inner pointer of a *temporary* `CString` allows the `CString`
/// to be dropped at the end of the statement, as it is not being referenced as far as the typesystem
/// is concerned. This means outside of the statement the pointer will point to freed memory, which
/// causes undefined behavior if the pointer is later dereferenced.
/// to be dropped at the end of the statement, as it is not being referenced as far as the
/// typesystem is concerned. This means outside of the statement the pointer will point to
/// freed memory, which causes undefined behavior if the pointer is later dereferenced.
pub TEMPORARY_CSTRING_AS_PTR,
Warn,
"detects getting the inner pointer of a temporary `CString`"

2
compiler/rustc_lint/src/non_local_def.rs
View File

@ -306,7 +306,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalDefinitions {
}
}
// Detecting if the impl definition is leaking outside of it's defining scope.
// Detecting if the impl definition is leaking outside of its defining scope.
//
// Rule: for each impl, instantiate all local types with inference vars and
// then assemble candidates for that goal, if there are more than 1 (non-private

10
compiler/rustc_lint/src/nonstandard_style.rs
View File

@ -297,14 +297,14 @@ impl NonSnakeCase {
// We cannot provide meaningful suggestions
// if the characters are in the category of "Uppercase Letter".
let sub = if name != sc {
// We have a valid span in almost all cases, but we don't have one when linting a crate
// name provided via the command line.
// We have a valid span in almost all cases, but we don't have one when linting a
// crate name provided via the command line.
if !span.is_dummy() {
let sc_ident = Ident::from_str_and_span(&sc, span);
if sc_ident.is_reserved() {
// We shouldn't suggest a reserved identifier to fix non-snake-case identifiers.
// Instead, recommend renaming the identifier entirely or, if permitted,
// escaping it to create a raw identifier.
// We shouldn't suggest a reserved identifier to fix non-snake-case
// identifiers. Instead, recommend renaming the identifier entirely or, if
// permitted, escaping it to create a raw identifier.
if sc_ident.name.can_be_raw() {
NonSnakeCaseDiagSub::RenameOrConvertSuggestion {
span,

4
compiler/rustc_lint/src/passes.rs
View File

@ -237,5 +237,5 @@ macro_rules! declare_combined_early_lint_pass {
}
/// A lint pass boxed up as a trait object.
pub type EarlyLintPassObject = Box<dyn EarlyLintPass + 'static>;
pub type LateLintPassObject<'tcx> = Box<dyn LateLintPass<'tcx> + 'tcx>;
pub(crate) type EarlyLintPassObject = Box<dyn EarlyLintPass + 'static>;
pub(crate) type LateLintPassObject<'tcx> = Box<dyn LateLintPass<'tcx> + 'tcx>;

9
compiler/rustc_lint/src/unused.rs
View File

@ -387,7 +387,8 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
}
}
// Returns whether further errors should be suppressed because either a lint has been emitted or the type should be ignored.
// Returns whether further errors should be suppressed because either a lint has been
// emitted or the type should be ignored.
fn check_must_use_def(
cx: &LateContext<'_>,
def_id: DefId,
@ -677,7 +678,8 @@ trait UnusedDelimLint {
return true;
}
// Check if LHS needs parens to prevent false-positives in cases like `fn x() -> u8 { ({ 0 } + 1) }`.
// Check if LHS needs parens to prevent false-positives in cases like
// `fn x() -> u8 { ({ 0 } + 1) }`.
//
// FIXME: https://github.com/rust-lang/rust/issues/119426
// The syntax tree in this code is from after macro expansion, so the
@ -722,7 +724,8 @@ trait UnusedDelimLint {
}
}
// Check if RHS needs parens to prevent false-positives in cases like `if (() == return) {}`.
// Check if RHS needs parens to prevent false-positives in cases like `if (() == return)
// {}`.
if !followed_by_block {
return false;
}