Rollup merge of #128919 - Nadrieril:lint-query-leaks, r=cjgillot

Add an internal lint that warns when accessing untracked data

Some methods access data that is not tracked by the query system and should be used with caution. As suggested in #128815 (comment), in this PR I propose a lint (modeled on the `potential_query_instability` lint) that warns when using some specially-annotatted functions.

I can't tell myself if this lint would be that useful, compared to renaming `Steal::is_stolen` to `is_stolen_untracked`. This would depend on whether there are other functions we'd want to lint like this. So far it seems they're called `*_untracked`, which may be clear enough.

r? ``@oli-obk``

compiler/rustc_data_structures/src/steal.rs

@@ -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()
     }

compiler/rustc_feature/src/builtin_attrs.rs

@@ -793,6 +793,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!(

compiler/rustc_lint/messages.ftl

@@ -699,6 +699,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

compiler/rustc_lint/src/internal.rs

@@ -17,8 +17,8 @@ use tracing::debug;
 
 use crate::lints::{
     BadOptAccessDiag, DefaultHashTypesDiag, DiagOutOfImpl, LintPassByHand, NonExistentDocKeyword,
-    NonGlobImportTypeIrInherent, QueryInstability, SpanUseEqCtxtDiag, TyQualified, TykindDiag,
-    TykindKind, TypeIrInherentUsage, UntranslatableDiag,
+    NonGlobImportTypeIrInherent, QueryInstability, QueryUntracked, SpanUseEqCtxtDiag, TyQualified,
+    TykindDiag, TykindKind, TypeIrInherentUsage, UntranslatableDiag,
 };
 use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
 
@@ -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<'_>) {
@@ -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) },
+                );
+            }
         }
     }
 }

compiler/rustc_lint/src/lib.rs

@@ -609,6 +609,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),

compiler/rustc_lint/src/lints.rs

@@ -894,6 +894,13 @@ pub(crate) struct QueryInstability {
     pub query: Symbol,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(lint_query_untracked)]
+#[note]
+pub(crate) struct QueryUntracked {
+    pub method: Symbol,
+}
+
 #[derive(LintDiagnostic)]
 #[diag(lint_span_use_eq_ctxt)]
 pub(crate) struct SpanUseEqCtxtDiag;

compiler/rustc_passes/src/check_attr.rs

@@ -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, ..] => {
@@ -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, ..] => {
@@ -452,11 +457,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,
@@ -1635,30 +1635,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 {

compiler/rustc_span/src/symbol.rs

@@ -1654,6 +1654,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,

src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs

@@ -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),

tests/ui-fulldeps/internal-lints/query_completeness.rs

@@ -0,0 +1,16 @@
+//@ compile-flags: -Z unstable-options
+// #[cfg(bootstrap)]: We can stop ignoring next beta bump; afterward this ALWAYS should run.
+//@ ignore-stage1 (requires matching sysroot built with in-tree compiler)
+#![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() {}

tests/ui-fulldeps/internal-lints/query_completeness.stderr

@@ -0,0 +1,15 @@
+error: `is_stolen` accesses information that is not tracked by the query system
+  --> $DIR/query_completeness.rs:12: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:5:9
+   |
+LL | #![deny(rustc::untracked_query_information)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+