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.

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

lint: add bad opt access internal lint #99710

Merged
merged 3 commits into from
Jul 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions compiler/rustc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ impl Callbacks for TimePassesCallbacks {
fn config(&mut self, config: &mut interface::Config) {
// If a --prints=... option has been given, we don't print the "total"
// time because it will mess up the --prints output. See #64339.
self.time_passes = config.opts.prints.is_empty()
&& (config.opts.unstable_opts.time_passes || config.opts.unstable_opts.time);
self.time_passes = config.opts.prints.is_empty() && config.opts.time_passes();
config.opts.trimmed_def_paths = TrimmedDefPaths::GoodPath;
}
}
Expand Down Expand Up @@ -249,7 +248,7 @@ fn run_compiler(
if sopts.describe_lints {
let mut lint_store = rustc_lint::new_lint_store(
sopts.unstable_opts.no_interleave_lints,
compiler.session().unstable_options(),
compiler.session().enable_internal_lints(),
);
let registered_lints =
if let Some(register_lints) = compiler.register_lints() {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/passes.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -256,3 +256,9 @@ passes-unused-duplicate = unused attribute
passes-unused-multiple = multiple `{$name}` attributes
.suggestion = remove this attribute
.note = attribute also specified here

passes-rustc-lint-opt-ty = `#[rustc_lint_opt_ty]` should be applied to a struct
.label = not a struct

passes-rustc-lint-opt-deny-field-access = `#[rustc_lint_opt_deny_field_access]` should be applied to a field
.label = not a field
6 changes: 6 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
// Used by the `rustc::untranslatable_diagnostic` and `rustc::diagnostic_outside_of_impl` lints
// to assist in changes to diagnostic APIs.
rustc_attr!(rustc_lint_diagnostics, Normal, template!(Word), WarnFollowing, INTERNAL_UNSTABLE),
// Used by the `rustc::bad_opt_access` lint to identify `DebuggingOptions` and `CodegenOptions`
// types (as well as any others in future).
rustc_attr!(rustc_lint_opt_ty, Normal, template!(Word), WarnFollowing, INTERNAL_UNSTABLE),
// Used by the `rustc::bad_opt_access` lint on fields
// types (as well as any others in future).
rustc_attr!(rustc_lint_opt_deny_field_access, Normal, template!(List: "message"), WarnFollowing, INTERNAL_UNSTABLE),

// ==========================================================================
// Internal attributes, Const related:
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,8 @@ pub fn create_compiler_and_run<R>(config: Config, f: impl FnOnce(&Compiler) -> R
})
}

// JUSTIFICATION: before session exists, only config
#[cfg_attr(not(bootstrap), allow(rustc::bad_opt_access))]
pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Send) -> R {
tracing::trace!("run_compiler");
util::run_in_thread_pool_with_globals(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ pub fn register_plugins<'a>(

let mut lint_store = rustc_lint::new_lint_store(
sess.opts.unstable_opts.no_interleave_lints,
sess.unstable_options(),
sess.enable_internal_lints(),
);
register_lints(sess, &mut lint_store);

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg_attr(not(bootstrap), allow(rustc::bad_opt_access))]
use crate::interface::parse_cfgspecs;

use rustc_data_structures::fx::FxHashSet;
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,8 @@ pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec<C
// Only check command line flags if present. If no types are specified by
// command line, then reuse the empty `base` Vec to hold the types that
// will be found in crate attributes.
// JUSTIFICATION: before wrapper fn is available
#[cfg_attr(not(bootstrap), allow(rustc::bad_opt_access))]
let mut base = session.opts.crate_types.clone();
if base.is_empty() {
base.extend(attr_types);
Expand Down
49 changes: 35 additions & 14 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,6 @@ fn typeck_results_of_method_fn<'tcx>(
cx: &LateContext<'tcx>,
expr: &Expr<'_>,
) -> Option<(Span, DefId, ty::subst::SubstsRef<'tcx>)> {
// FIXME(rustdoc): Lints which use this function use typecheck results which can cause
// `rustdoc` to error if there are resolution failures.
//
// As internal lints are currently always run if there are `unstable_options`, they are added
// to the lint store of rustdoc. Internal lints are also not used via the `lint_mod` query.
// Crate lints run outside of a query so rustdoc currently doesn't disable them.
//
// Instead of relying on this, either change crate lints to a query disabled by rustdoc, only
// run internal lints if the user is explicitly opting in or figure out a different way to
// avoid running lints for rustdoc.
if cx.tcx.sess.opts.actually_rustdoc {
return None;
}

match expr.kind {
ExprKind::MethodCall(segment, _, _)
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) =>
Expand Down Expand Up @@ -446,3 +432,38 @@ impl LateLintPass<'_> for Diagnostics {
}
}
}

declare_tool_lint! {
pub rustc::BAD_OPT_ACCESS,
Deny,
"prevent using options by field access when there is a wrapper function",
report_in_external_macro: true
}

declare_lint_pass!(BadOptAccess => [ BAD_OPT_ACCESS ]);

impl LateLintPass<'_> for BadOptAccess {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
let ExprKind::Field(base, target) = expr.kind else { return };
let Some(adt_def) = cx.typeck_results().expr_ty(base).ty_adt_def() else { return };
// Skip types without `#[rustc_lint_opt_ty]` - only so that the rest of the lint can be
// avoided.
if !cx.tcx.has_attr(adt_def.did(), sym::rustc_lint_opt_ty) {
return;
}

for field in adt_def.all_fields() {
if field.name == target.name &&
let Some(attr) = cx.tcx.get_attr(field.did, sym::rustc_lint_opt_deny_field_access) &&
let Some(items) = attr.meta_item_list() &&
let Some(item) = items.first() &&
let Some(literal) = item.literal() &&
let ast::LitKind::Str(val, _) = literal.kind
{
cx.struct_span_lint(BAD_OPT_ACCESS, expr.span, |lint| {
lint.build(val.as_str()).emit(); }
);
}
}
}
}
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,14 @@ fn register_internals(store: &mut LintStore) {
store.register_late_pass(|| Box::new(TyTyKind));
store.register_lints(&Diagnostics::get_lints());
store.register_late_pass(|| Box::new(Diagnostics));
store.register_lints(&BadOptAccess::get_lints());
store.register_late_pass(|| Box::new(BadOptAccess));
store.register_lints(&PassByValue::get_lints());
store.register_late_pass(|| Box::new(PassByValue));
// 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
// and translation is completed
store.register_group(
false,
"rustc::internal",
Expand All @@ -523,6 +529,7 @@ fn register_internals(store: &mut LintStore) {
LintId::of(LINT_PASS_IMPL_WITHOUT_MACRO),
LintId::of(USAGE_OF_QUALIFIED_TY),
LintId::of(EXISTING_DOC_KEYWORD),
LintId::of(BAD_OPT_ACCESS),
],
);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/lower_slice_len.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub struct LowerSliceLenCalls;

impl<'tcx> MirPass<'tcx> for LowerSliceLenCalls {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
sess.opts.mir_opt_level() > 0
sess.mir_opt_level() > 0
}

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/reveal_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub struct RevealAll;

impl<'tcx> MirPass<'tcx> for RevealAll {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
sess.opts.mir_opt_level() >= 3 || super::inline::Inline.is_enabled(sess)
sess.mir_opt_level() >= 3 || super::inline::Inline.is_enabled(sess)
}

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
Expand Down
33 changes: 33 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ impl CheckAttrVisitor<'_> {
sym::rustc_lint_diagnostics => {
self.check_rustc_lint_diagnostics(&attr, span, target)
}
sym::rustc_lint_opt_ty => self.check_rustc_lint_opt_ty(&attr, span, target),
sym::rustc_lint_opt_deny_field_access => {
self.check_rustc_lint_opt_deny_field_access(&attr, span, target)
}
sym::rustc_clean
| sym::rustc_dirty
| sym::rustc_if_this_changed
Expand Down Expand Up @@ -1382,6 +1386,35 @@ impl CheckAttrVisitor<'_> {
self.check_applied_to_fn_or_method(attr, span, target)
}

/// Checks that the `#[rustc_lint_opt_ty]` attribute is only applied to a struct.
fn check_rustc_lint_opt_ty(&self, attr: &Attribute, span: Span, target: Target) -> bool {
match target {
Target::Struct => true,
_ => {
self.tcx.sess.emit_err(errors::RustcLintOptTy { attr_span: attr.span, span });
false
}
}
}

/// Checks that the `#[rustc_lint_opt_deny_field_access]` attribute is only applied to a field.
fn check_rustc_lint_opt_deny_field_access(
&self,
attr: &Attribute,
span: Span,
target: Target,
) -> bool {
match target {
Target::Field => true,
_ => {
self.tcx
.sess
.emit_err(errors::RustcLintOptDenyFieldAccess { attr_span: attr.span, span });
false
}
}
}

/// Checks that the dep-graph debugging attributes are only present when the query-dep-graph
/// option is passed to the compiler.
fn check_rustc_dirty_clean(&self, attr: &Attribute) -> bool {
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,3 +625,21 @@ pub struct UnusedMultiple {
pub other: Span,
pub name: Symbol,
}

#[derive(SessionDiagnostic)]
#[error(passes::rustc_lint_opt_ty)]
pub struct RustcLintOptTy {
#[primary_span]
pub attr_span: Span,
#[label]
pub span: Span,
}

#[derive(SessionDiagnostic)]
#[error(passes::rustc_lint_opt_deny_field_access)]
pub struct RustcLintOptDenyFieldAccess {
#[primary_span]
pub attr_span: Span,
#[label]
pub span: Span,
}
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,8 @@ fn default_configuration(sess: &Session) -> CrateConfig {
if sess.opts.debug_assertions {
ret.insert((sym::debug_assertions, None));
}
// JUSTIFICATION: before wrapper fn is available
#[cfg_attr(not(bootstrap), allow(rustc::bad_opt_access))]
if sess.opts.crate_types.contains(&CrateType::ProcMacro) {
ret.insert((sym::proc_macro, None));
}
Expand Down Expand Up @@ -2196,6 +2198,8 @@ fn parse_remap_path_prefix(
mapping
}

// JUSTIFICATION: before wrapper fn is available
#[cfg_attr(not(bootstrap), allow(rustc::bad_opt_access))]
pub fn build_session_options(matches: &getopts::Matches) -> Options {
let color = parse_color(matches);

Expand Down
Loading