Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Various style improvements to rustc_lint::levels #122416

Merged
merged 1 commit into from
Mar 14, 2024
Merged

Various style improvements to rustc_lint::levels #122416

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
249 changes: 121 additions & 128 deletions compiler/rustc_lint/src/levels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,12 @@ impl LintLevelSets {
mut idx: LintStackIndex,
aux: Option<&FxIndexMap<LintId, LevelAndSource>>,
) -> (Option<Level>, LintLevelSource) {
if let Some(specs) = aux {
if let Some(&(level, src)) = specs.get(&id) {
return (Some(level), src);
}
if let Some(specs) = aux
&& let Some(&(level, src)) = specs.get(&id)
{
return (Some(level), src);
}

loop {
let LintSet { ref specs, parent } = self.list[idx];
if let Some(&(level, src)) = specs.get(&id) {
Expand Down Expand Up @@ -177,7 +178,7 @@ fn shallow_lint_levels_on(tcx: TyCtxt<'_>, owner: hir::OwnerId) -> ShallowLintLe
// There is only something to do if there are attributes at all.
[] => {}
// Most of the time, there is only one attribute. Avoid fetching HIR in that case.
[(local_id, _)] => levels.add_id(HirId { owner, local_id: *local_id }),
&[(local_id, _)] => levels.add_id(HirId { owner, local_id }),
// Otherwise, we need to visit the attributes in source code order, so we fetch HIR and do
// a standard visit.
// FIXME(#102522) Just iterate on attrs once that iteration order matches HIR's.
Expand Down Expand Up @@ -643,63 +644,61 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
//
// This means that this only errors if we're truly lowering the lint
// level from forbid.
if self.lint_added_lints && level != Level::Forbid {
if let Level::Forbid = old_level {
// Backwards compatibility check:
//
// We used to not consider `forbid(lint_group)`
// as preventing `allow(lint)` for some lint `lint` in
// `lint_group`. For now, issue a future-compatibility
// warning for this case.
let id_name = id.lint.name_lower();
let fcw_warning = match old_src {
LintLevelSource::Default => false,
LintLevelSource::Node { name, .. } => self.store.is_lint_group(name),
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
};
debug!(
"fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
fcw_warning,
self.current_specs(),
old_src,
id_name
);
let sub = match old_src {
LintLevelSource::Default => {
OverruledAttributeSub::DefaultSource { id: id.to_string() }
}
LintLevelSource::Node { span, reason, .. } => {
OverruledAttributeSub::NodeSource { span, reason }
}
LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource,
};
if !fcw_warning {
self.sess.dcx().emit_err(OverruledAttribute {
span: src.span(),
if self.lint_added_lints && level != Level::Forbid && old_level == Level::Forbid {
// Backwards compatibility check:
//
// We used to not consider `forbid(lint_group)`
// as preventing `allow(lint)` for some lint `lint` in
// `lint_group`. For now, issue a future-compatibility
// warning for this case.
let id_name = id.lint.name_lower();
let fcw_warning = match old_src {
LintLevelSource::Default => false,
LintLevelSource::Node { name, .. } => self.store.is_lint_group(name),
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
};
debug!(
"fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
fcw_warning,
self.current_specs(),
old_src,
id_name
);
let sub = match old_src {
LintLevelSource::Default => {
OverruledAttributeSub::DefaultSource { id: id.to_string() }
}
LintLevelSource::Node { span, reason, .. } => {
OverruledAttributeSub::NodeSource { span, reason }
}
LintLevelSource::CommandLine(_, _) => OverruledAttributeSub::CommandLineSource,
};
if !fcw_warning {
self.sess.dcx().emit_err(OverruledAttribute {
span: src.span(),
overruled: src.span(),
lint_level: level.as_str(),
lint_source: src.name(),
sub,
});
} else {
self.emit_span_lint(
FORBIDDEN_LINT_GROUPS,
src.span().into(),
OverruledAttributeLint {
overruled: src.span(),
lint_level: level.as_str(),
lint_source: src.name(),
sub,
});
} else {
self.emit_span_lint(
FORBIDDEN_LINT_GROUPS,
src.span().into(),
OverruledAttributeLint {
overruled: src.span(),
lint_level: level.as_str(),
lint_source: src.name(),
sub,
},
);
}
},
);
}

// Retain the forbid lint level, unless we are
// issuing a FCW. In the FCW case, we want to
// respect the new setting.
if !fcw_warning {
return;
}
// Retain the forbid lint level, unless we are
// issuing a FCW. In the FCW case, we want to
// respect the new setting.
if !fcw_warning {
return;
}
}

Expand Down Expand Up @@ -770,15 +769,15 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {

let Some(mut metas) = attr.meta_item_list() else { continue };

if metas.is_empty() {
// Check whether `metas` is empty, and get its last element.
let Some(tail_li) = metas.last() else {
// This emits the unused_attributes lint for `#[level()]`
continue;
}
};

// Before processing the lint names, look for a reason (RFC 2383)
// at the end.
let mut reason = None;
let tail_li = &metas[metas.len() - 1];
if let Some(item) = tail_li.meta_item() {
match item.kind {
ast::MetaItemKind::Word => {} // actual lint names handled later
Expand Down Expand Up @@ -834,21 +833,16 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
let meta_item = match li {
ast::NestedMetaItem::MetaItem(meta_item) if meta_item.is_word() => meta_item,
_ => {
if let Some(item) = li.meta_item() {
if let ast::MetaItemKind::NameValue(_) = item.kind {
if item.path == sym::reason {
sess.dcx().emit_err(MalformedAttribute {
span: sp,
sub: MalformedAttributeSub::ReasonMustComeLast(sp),
});
continue;
}
}
}
sess.dcx().emit_err(MalformedAttribute {
span: sp,
sub: MalformedAttributeSub::BadAttributeArgument(sp),
});
let sub = if let Some(item) = li.meta_item()
&& let ast::MetaItemKind::NameValue(_) = item.kind
&& item.path == sym::reason
{
MalformedAttributeSub::ReasonMustComeLast(sp)
} else {
MalformedAttributeSub::BadAttributeArgument(sp)
};

sess.dcx().emit_err(MalformedAttribute { span: sp, sub });
continue;
}
};
Expand Down Expand Up @@ -987,11 +981,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
}

CheckLintNameResult::NoLint(suggestion) => {
let name = if let Some(tool_ident) = tool_ident {
format!("{}::{}", tool_ident.name, name)
} else {
name.to_string()
};
let name = tool_ident.map(|tool| format!("{tool}::{name}")).unwrap_or(name);
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
let suggestion = suggestion.map(|(replace, from_rustc)| {
UnknownLintSuggestion::WithSpan { suggestion: sp, replace, from_rustc }
});
Expand All @@ -1005,27 +995,24 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
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.
if let CheckLintNameResult::Ok(ids) =
let CheckLintNameResult::Ok(ids) =
self.store.check_lint_name(&new_name, None, self.registered_tools)
{
let src = LintLevelSource::Node {
name: Symbol::intern(&new_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),
);
}
} else {
else {
panic!("renamed lint does not exist: {new_name}");
};

let src =
LintLevelSource::Node { name: Symbol::intern(&new_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),
);
}
}
}
Expand Down Expand Up @@ -1058,38 +1045,44 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
/// Returns `true` if the lint's feature is enabled.
#[track_caller]
fn check_gated_lint(&self, lint_id: LintId, span: Span, lint_from_cli: bool) -> bool {
if let Some(feature) = lint_id.lint.feature_gate {
if !self.features.active(feature) {
if self.lint_added_lints {
let lint = builtin::UNKNOWN_LINTS;
let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS);
// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
lint_level(
self.sess,
let feature = if let Some(feature) = lint_id.lint.feature_gate
&& !self.features.active(feature)
{
// Lint is behind a feature that is not enabled; eventually return false.
feature
} else {
// Lint is ungated or its feature is enabled; exit early.
return true;
};

if self.lint_added_lints {
let lint = builtin::UNKNOWN_LINTS;
let (level, src) = self.lint_level(builtin::UNKNOWN_LINTS);
// FIXME: make this translatable
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
lint_level(
self.sess,
lint,
level,
src,
Some(span.into()),
fluent::lint_unknown_gated_lint,
|lint| {
lint.arg("name", lint_id.lint.name_lower());
lint.note(fluent::lint_note);
rustc_session::parse::add_feature_diagnostics_for_issue(
lint,
level,
src,
Some(span.into()),
fluent::lint_unknown_gated_lint,
|lint| {
lint.arg("name", lint_id.lint.name_lower());
lint.note(fluent::lint_note);
rustc_session::parse::add_feature_diagnostics_for_issue(
lint,
&self.sess,
feature,
GateIssue::Language,
lint_from_cli,
);
},
&self.sess,
feature,
GateIssue::Language,
lint_from_cli,
);
}
return false;
}
},
);
}
true

false
}

/// Find the lint level for a lint.
Expand Down
Loading