about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-09-05 19:43:46 +0200
committerGitHub <noreply@github.com>2024-09-05 19:43:46 +0200
commit46f390f047309c943a7b773ae1f4c225a8d65907 (patch)
treeec86d99ba6779d65399885e6a9d42e92a75fc5c4
parenteb33b43bab08223fa6b46abacc1e95e859fe375d (diff)
parent040239465aefc09f8c837ff9d4d66ca6297e5c5c (diff)
downloadrust-46f390f047309c943a7b773ae1f4c225a8d65907.tar.gz
rust-46f390f047309c943a7b773ae1f4c225a8d65907.zip
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 https://github.com/rust-lang/rust/pull/128815#issuecomment-2275488683, 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``
-rw-r--r--compiler/rustc_data_structures/src/steal.rs1
-rw-r--r--compiler/rustc_feature/src/builtin_attrs.rs6
-rw-r--r--compiler/rustc_lint/messages.ftl3
-rw-r--r--compiler/rustc_lint/src/internal.rs24
-rw-r--r--compiler/rustc_lint/src/lib.rs1
-rw-r--r--compiler/rustc_lint/src/lints.rs7
-rw-r--r--compiler/rustc_passes/src/check_attr.rs40
-rw-r--r--compiler/rustc_span/src/symbol.rs1
-rw-r--r--src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs3
-rw-r--r--tests/ui-fulldeps/internal-lints/query_completeness.rs16
-rw-r--r--tests/ui-fulldeps/internal-lints/query_completeness.stderr15
11 files changed, 82 insertions, 35 deletions
diff --git a/compiler/rustc_data_structures/src/steal.rs b/compiler/rustc_data_structures/src/steal.rs
index 0f2c0eee27d..aaa95f6b7f1 100644
--- a/compiler/rustc_data_structures/src/steal.rs
+++ b/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()
     }
diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs
index e2491922b8d..e86421f2150 100644
--- a/compiler/rustc_feature/src/builtin_attrs.rs
+++ b/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!(
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
index 35334595833..759320b9eb6 100644
--- a/compiler/rustc_lint/messages.ftl
+++ b/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
diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs
index 2e8116b8ba8..9d637c1eb7f 100644
--- a/compiler/rustc_lint/src/internal.rs
+++ b/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) },
+                );
+            }
         }
     }
 }
diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs
index c5a5c5b30af..105a90de034 100644
--- a/compiler/rustc_lint/src/lib.rs
+++ b/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),
diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
index 7ca282b7c85..9050f36acba 100644
--- a/compiler/rustc_lint/src/lints.rs
+++ b/compiler/rustc_lint/src/lints.rs
@@ -895,6 +895,13 @@ pub(crate) struct QueryInstability {
 }
 
 #[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;
 
diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs
index 21478a44b0e..e41f89a3c9d 100644
--- a/compiler/rustc_passes/src/check_attr.rs
+++ b/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 {
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
index 36b31d10c4d..418d1078900 100644
--- a/compiler/rustc_span/src/symbol.rs
+++ b/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,
diff --git a/src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs b/src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs
index ee15b1b5ce9..5c25a55362e 100644
--- a/src/tools/rust-analyzer/crates/hir-expand/src/inert_attr_macro.rs
+++ b/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),
diff --git a/tests/ui-fulldeps/internal-lints/query_completeness.rs b/tests/ui-fulldeps/internal-lints/query_completeness.rs
new file mode 100644
index 00000000000..50b0fb4c3fc
--- /dev/null
+++ b/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() {}
diff --git a/tests/ui-fulldeps/internal-lints/query_completeness.stderr b/tests/ui-fulldeps/internal-lints/query_completeness.stderr
new file mode 100644
index 00000000000..35bb867f40e
--- /dev/null
+++ b/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
+