Skip to content

Commit

Permalink
Fix up comments.
Browse files Browse the repository at this point in the history
Wrap overly long ones, etc.
  • Loading branch information
nnethercote committed May 13, 2024
1 parent 023d1b9 commit 92b9270
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 51 deletions.
42 changes: 26 additions & 16 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1493,8 +1493,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;
}

Expand Down Expand Up @@ -2209,7 +2210,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 {
Expand Down Expand Up @@ -2586,7 +2588,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,
Expand All @@ -2596,10 +2599,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)
Expand Down Expand Up @@ -2810,7 +2813,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
Expand All @@ -2823,7 +2827,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;
};

Expand All @@ -2840,12 +2845,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;
}

Expand All @@ -2858,7 +2866,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;
}
}
Expand All @@ -2877,7 +2886,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() };

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub struct OverruledAttribute<'a> {
#[subdiagnostic]
pub sub: OverruledAttributeSub,
}
//

pub enum OverruledAttributeSub {
DefaultSource { id: String },
NodeSource { span: Span, reason: Option<Symbol> },
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_lint/src/let_underscore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
22 changes: 13 additions & 9 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,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")
Expand All @@ -758,8 +759,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),
};

Expand Down Expand Up @@ -858,13 +859,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,
Expand Down Expand Up @@ -995,7 +998,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 {
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_lint/src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`"
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/non_local_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,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
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_lint/src/nonstandard_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 92b9270

Please sign in to comment.