Skip to content

Commit

Permalink
Add an internal lint that warns when accessing untracked data
Browse files Browse the repository at this point in the history
  • Loading branch information
Nadrieril committed Aug 10, 2024
1 parent 19469cb commit 0810a03
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 35 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/steal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ impl<T> Steal<T> {
///
/// This should not be used within rustc as it leaks information not tracked
/// by the query system, breaking incremental compilation.
#[cfg_attr(not(bootstrap), rustc_lint_untracked_query_information)]
pub fn is_stolen(&self) -> bool {
self.value.borrow().is_none()
}
Expand Down
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 @@ -792,6 +792,12 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_lint_query_instability, Normal, template!(Word),
WarnFollowing, EncodeCrossCrate::Yes, INTERNAL_UNSTABLE
),
// Used by the `rustc::untracked_query_information` lint to warn methods which
// might not be stable during incremental compilation.
rustc_attr!(
rustc_lint_untracked_query_information, Normal, template!(Word),
WarnFollowing, EncodeCrossCrate::Yes, INTERNAL_UNSTABLE
),
// Used by the `rustc::diagnostic_outside_of_impl` lints to assist in changes to diagnostic
// APIs. Any function with this attribute will be checked by that lint.
rustc_attr!(
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,9 @@ lint_ptr_null_checks_ref = references are not nullable, so checking them for nul
lint_query_instability = using `{$query}` can result in unstable query results
.note = if you believe this case to be fine, allow this lint and add a comment explaining your rationale
lint_query_untracked = `{$method}` accesses information that is not tracked by the query system
.note = if you believe this case to be fine, allow this lint and add a comment explaining your rationale
lint_range_endpoint_out_of_range = range endpoint is out of range for `{$ty}`
lint_range_use_inclusive_range = use an inclusive range instead
Expand Down
24 changes: 21 additions & 3 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use tracing::debug;

use crate::lints::{
BadOptAccessDiag, DefaultHashTypesDiag, DiagOutOfImpl, LintPassByHand, NonExistentDocKeyword,
NonGlobImportTypeIrInherent, QueryInstability, SpanUseEqCtxtDiag, TyQualified, TykindDiag,
TykindKind, UntranslatableDiag,
NonGlobImportTypeIrInherent, QueryInstability, QueryUntracked, SpanUseEqCtxtDiag, TyQualified,
TykindDiag, TykindKind, UntranslatableDiag,
};
use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};

Expand Down Expand Up @@ -88,7 +88,18 @@ declare_tool_lint! {
report_in_external_macro: true
}

declare_lint_pass!(QueryStability => [POTENTIAL_QUERY_INSTABILITY]);
declare_tool_lint! {
/// The `untracked_query_information` lint detects use of methods which leak information not
/// tracked by the query system, such as whether a `Steal<T>` value has already been stolen. In
/// order not to break incremental compilation, such methods must be used very carefully or not
/// at all.
pub rustc::UNTRACKED_QUERY_INFORMATION,
Allow,
"require explicit opt-in when accessing information not tracked by the query system",
report_in_external_macro: true
}

declare_lint_pass!(QueryStability => [POTENTIAL_QUERY_INSTABILITY, UNTRACKED_QUERY_INFORMATION]);

impl LateLintPass<'_> for QueryStability {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
Expand All @@ -102,6 +113,13 @@ impl LateLintPass<'_> for QueryStability {
QueryInstability { query: cx.tcx.item_name(def_id) },
);
}
if cx.tcx.has_attr(def_id, sym::rustc_lint_untracked_query_information) {
cx.emit_span_lint(
UNTRACKED_QUERY_INFORMATION,
span,
QueryUntracked { method: cx.tcx.item_name(def_id) },
);
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,7 @@ fn register_internals(store: &mut LintStore) {
vec![
LintId::of(DEFAULT_HASH_TYPES),
LintId::of(POTENTIAL_QUERY_INSTABILITY),
LintId::of(UNTRACKED_QUERY_INFORMATION),
LintId::of(USAGE_OF_TY_TYKIND),
LintId::of(PASS_BY_VALUE),
LintId::of(LINT_PASS_IMPL_WITHOUT_MACRO),
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,13 @@ pub struct QueryInstability {
pub query: Symbol,
}

#[derive(LintDiagnostic)]
#[diag(lint_query_untracked)]
#[note]
pub struct QueryUntracked {
pub method: Symbol,
}

#[derive(LintDiagnostic)]
#[diag(lint_span_use_eq_ctxt)]
pub struct SpanUseEqCtxtDiag;
Expand Down
40 changes: 8 additions & 32 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,9 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
[sym::inline, ..] => self.check_inline(hir_id, attr, span, target),
[sym::coverage, ..] => self.check_coverage(attr, span, target),
[sym::optimize, ..] => self.check_optimize(hir_id, attr, target),
[sym::no_sanitize, ..] => self.check_no_sanitize(hir_id, attr, span, target),
[sym::no_sanitize, ..] => {
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}
[sym::non_exhaustive, ..] => self.check_non_exhaustive(hir_id, attr, span, target),
[sym::marker, ..] => self.check_marker(hir_id, attr, span, target),
[sym::target_feature, ..] => {
Expand Down Expand Up @@ -166,10 +168,13 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.check_rustc_legacy_const_generics(hir_id, attr, span, target, item)
}
[sym::rustc_lint_query_instability, ..] => {
self.check_rustc_lint_query_instability(hir_id, attr, span, target)
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}
[sym::rustc_lint_untracked_query_information, ..] => {
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}
[sym::rustc_lint_diagnostics, ..] => {
self.check_rustc_lint_diagnostics(hir_id, attr, span, target)
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}
[sym::rustc_lint_opt_ty, ..] => self.check_rustc_lint_opt_ty(attr, span, target),
[sym::rustc_lint_opt_deny_field_access, ..] => {
Expand Down Expand Up @@ -451,11 +456,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

/// Checks that `#[no_sanitize(..)]` is applied to a function or method.
fn check_no_sanitize(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) {
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}

fn check_generic_attr(
&self,
hir_id: HirId,
Expand Down Expand Up @@ -1603,30 +1603,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

/// Checks that the `#[rustc_lint_query_instability]` attribute is only applied to a function
/// or method.
fn check_rustc_lint_query_instability(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
) {
self.check_applied_to_fn_or_method(hir_id, attr, span, target)
}

/// Checks that the `#[rustc_lint_diagnostics]` attribute is only applied to a function or
/// method.
fn check_rustc_lint_diagnostics(
&self,
hir_id: HirId,
attr: &Attribute,
span: Span,
target: Target,
) {
self.check_applied_to_fn_or_method(hir_id, 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) {
match target {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,7 @@ symbols! {
rustc_lint_opt_deny_field_access,
rustc_lint_opt_ty,
rustc_lint_query_instability,
rustc_lint_untracked_query_information,
rustc_macro_transparency,
rustc_main,
rustc_mir,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,9 @@ pub const INERT_ATTRIBUTES: &[BuiltinAttribute] = &[
// Used by the `rustc::potential_query_instability` lint to warn methods which
// might not be stable during incremental compilation.
rustc_attr!(rustc_lint_query_instability, Normal, template!(Word), WarnFollowing, INTERNAL_UNSTABLE),
// Used by the `rustc::untracked_query_information` lint to warn methods which
// might break incremental compilation.
rustc_attr!(rustc_lint_untracked_query_information, Normal, template!(Word), WarnFollowing, INTERNAL_UNSTABLE),
// 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),
Expand Down
14 changes: 14 additions & 0 deletions tests/ui-fulldeps/internal-lints/query_completeness.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ compile-flags: -Z unstable-options
#![feature(rustc_private)]
#![deny(rustc::untracked_query_information)]

extern crate rustc_data_structures;

use rustc_data_structures::steal::Steal;

fn use_steal(x: Steal<()>) {
let _ = x.is_stolen();
//~^ ERROR `is_stolen` accesses information that is not tracked by the query system
}

fn main() {}
15 changes: 15 additions & 0 deletions tests/ui-fulldeps/internal-lints/query_completeness.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: `is_stolen` accesses information that is not tracked by the query system
--> $DIR/query_completeness.rs:10:15
|
LL | let _ = x.is_stolen();
| ^^^^^^^^^
|
= note: if you believe this case to be fine, allow this lint and add a comment explaining your rationale
note: the lint level is defined here
--> $DIR/query_completeness.rs:3:9
|
LL | #![deny(rustc::untracked_query_information)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

0 comments on commit 0810a03

Please sign in to comment.