Skip to content

Commit

Permalink
lint: add bad opt access internal lint
Browse files Browse the repository at this point in the history
Some command-line options accessible through `sess.opts` are best
accessed through wrapper functions on `Session`, `TyCtxt` or otherwise,
rather than through field access on the option struct in the `Session`.

Adds a new lint which triggers on those options that should be accessed
through a wrapper function so that this is prohibited. Options are
annotated with a new attribute `rustc_lint_opt_deny_field_access` which
can specify the error message (i.e. "use this other function instead")
to be emitted.

A simpler alternative would be to simply rename the options in the
option type so that it is clear they should not be used, however this
doesn't prevent uses, just discourages them. Another alternative would
be to make the option fields private, and adding accessor functions on
the option types, however the wrapper functions sometimes rely on
additional state from `Session` or `TyCtxt` which wouldn't be available
in an function on the option type, so the accessor would simply make the
field available and its use would be discouraged too.

Signed-off-by: David Wood <david.wood@huawei.com>
  • Loading branch information
davidtwco committed Jul 27, 2022
1 parent f5e005f commit 7bab769
Show file tree
Hide file tree
Showing 19 changed files with 553 additions and 344 deletions.
3 changes: 1 addition & 2 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
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
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
35 changes: 35 additions & 0 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,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(); }
);
}
}
}
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ 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
Expand All @@ -527,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

0 comments on commit 7bab769

Please sign in to comment.