Auto merge of rust-lang#88240 - GuillaumeGomez:rollup-wdom91m, r=GuillaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#86747 (Improve wording of the `drop_bounds` lint)
 - rust-lang#87166 (Show discriminant before overflow in diagnostic for duplicate values.)
 - rust-lang#88077 (Generate an iOS LLVM target with a specific version)
 - rust-lang#88164 (PassWrapper: adapt for LLVM 14 changes)
 - rust-lang#88211 (cleanup: `Span::new` -> `Span::with_lo`)
 - rust-lang#88229 (Suggest importing the right kind of macro.)
 - rust-lang#88238 (Stop tracking namespace in used_imports.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

compiler/rustc_lint/src/traits.rs

@@ -18,23 +18,27 @@ declare_lint! {
     ///
     /// ### Explanation
     ///
-    /// `Drop` bounds do not really accomplish anything. A type may have
-    /// compiler-generated drop glue without implementing the `Drop` trait
-    /// itself. The `Drop` trait also only has one method, `Drop::drop`, and
-    /// that function is by fiat not callable in user code. So there is really
-    /// no use case for using `Drop` in trait bounds.
+    /// A generic trait bound of the form `T: Drop` is most likely misleading
+    /// and not what the programmer intended (they probably should have used
+    /// `std::mem::needs_drop` instead).
     ///
-    /// The most likely use case of a drop bound is to distinguish between
-    /// types that have destructors and types that don't. Combined with
-    /// specialization, a naive coder would write an implementation that
-    /// assumed a type could be trivially dropped, then write a specialization
-    /// for `T: Drop` that actually calls the destructor. Except that doing so
-    /// is not correct; String, for example, doesn't actually implement Drop,
-    /// but because String contains a Vec, assuming it can be trivially dropped
-    /// will leak memory.
+    /// `Drop` bounds do not actually indicate whether a type can be trivially
+    /// dropped or not, because a composite type containing `Drop` types does
+    /// not necessarily implement `Drop` itself. Naïvely, one might be tempted
+    /// to write an implementation that assumes that a type can be trivially
+    /// dropped while also supplying a specialization for `T: Drop` that
+    /// actually calls the destructor. However, this breaks down e.g. when `T`
+    /// is `String`, which does not implement `Drop` itself but contains a
+    /// `Vec`, which does implement `Drop`, so assuming `T` can be trivially
+    /// dropped would lead to a memory leak here.
+    ///
+    /// Furthermore, the `Drop` trait only contains one method, `Drop::drop`,
+    /// which may not be called explicitly in user code (`E0040`), so there is
+    /// really no use case for using `Drop` in trait bounds, save perhaps for
+    /// some obscure corner cases, which can use `#[allow(drop_bounds)]`.
     pub DROP_BOUNDS,
     Warn,
-    "bounds of the form `T: Drop` are useless"
+    "bounds of the form `T: Drop` are most likely incorrect"
 }
 
 declare_lint! {
@@ -102,8 +106,8 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
                         None => return,
                     };
                     let msg = format!(
-                        "bounds on `{}` are useless, consider instead \
-                         using `{}` to detect if a type has a destructor",
+                        "bounds on `{}` are most likely incorrect, consider instead \
+                         using `{}` to detect whether a type can be trivially dropped",
                         predicate,
                         cx.tcx.def_path_str(needs_drop)
                     );

compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

@@ -922,9 +922,17 @@ LLVMRustOptimizeWithNewPassManager(
           MPM.addPass(RequireAnalysisPass<ASanGlobalsMetadataAnalysis, Module>());
           MPM.addPass(ModuleAddressSanitizerPass(
               /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover));
+#if LLVM_VERSION_GE(14, 0)
+          AddressSanitizerOptions opts(/*CompileKernel=*/false,
+                                       SanitizerOptions->SanitizeAddressRecover,
+                                       /*UseAfterScope=*/true,
+                                       AsanDetectStackUseAfterReturnMode::Runtime);
+          MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass(opts)));
+#else
           MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass(
               /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover,
               /*UseAfterScope=*/true)));
+#endif
         }
       );
 #else
@@ -952,8 +960,15 @@ LLVMRustOptimizeWithNewPassManager(
 #if LLVM_VERSION_GE(11, 0)
       OptimizerLastEPCallbacks.push_back(
         [SanitizerOptions](ModulePassManager &MPM, OptimizationLevel Level) {
+#if LLVM_VERSION_GE(14, 0)
+          HWAddressSanitizerOptions opts(
+              /*CompileKernel=*/false, SanitizerOptions->SanitizeHWAddressRecover,
+              /*DisableOptimization=*/false);
+          MPM.addPass(HWAddressSanitizerPass(opts));
+#else
           MPM.addPass(HWAddressSanitizerPass(
               /*CompileKernel=*/false, SanitizerOptions->SanitizeHWAddressRecover));
+#endif
         }
       );
 #else

compiler/rustc_middle/src/middle/region.rs

@@ -189,7 +189,7 @@ impl Scope {
                 // To avoid issues with macro-generated spans, the span
                 // of the statement must be nested in that of the block.
                 if span.lo() <= stmt_span.lo() && stmt_span.lo() <= span.hi() {
-                    return Span::new(stmt_span.lo(), span.hi(), span.ctxt());
+                    return span.with_lo(stmt_span.lo());
                 }
             }
         }

compiler/rustc_resolve/src/check_unused.rs

@@ -63,8 +63,7 @@ impl<'a, 'b> UnusedImportCheckVisitor<'a, 'b> {
     // We have information about whether `use` (import) items are actually
     // used now. If an import is not used at all, we signal a lint error.
     fn check_import(&mut self, id: ast::NodeId) {
-        let mut used = false;
-        self.r.per_ns(|this, ns| used |= this.used_imports.contains(&(id, ns)));
+        let used = self.r.used_imports.contains(&id);
         let def_id = self.r.local_def_id(id);
         if !used {
             if self.r.maybe_unused_trait_imports.contains(&def_id) {

compiler/rustc_resolve/src/diagnostics.rs

@@ -950,9 +950,7 @@ impl<'a> Resolver<'a> {
         self.add_typo_suggestion(err, suggestion, ident.span);
 
         let import_suggestions =
-            self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, |res| {
-                matches!(res, Res::Def(DefKind::Macro(MacroKind::Bang), _))
-            });
+            self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
         show_candidates(err, None, &import_suggestions, false, true);
 
         if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {

compiler/rustc_resolve/src/imports.rs

@@ -303,7 +303,7 @@ impl<'a> Resolver<'a> {
                     if self.last_import_segment && check_usable(self, binding).is_err() {
                         Err((Determined, Weak::No))
                     } else {
-                        self.record_use(ident, ns, binding, restricted_shadowing);
+                        self.record_use(ident, binding, restricted_shadowing);
 
                         if let Some(shadowed_glob) = resolution.shadowed_glob {
                             // Forbid expanded shadowing to avoid time travel.
@@ -609,9 +609,9 @@ impl<'a> Resolver<'a> {
             self.per_ns(|this, ns| {
                 let key = this.new_key(target, ns);
                 let _ = this.try_define(import.parent_scope.module, key, dummy_binding);
-                // Consider erroneous imports used to avoid duplicate diagnostics.
-                this.record_use(target, ns, dummy_binding, false);
             });
+            // Consider erroneous imports used to avoid duplicate diagnostics.
+            self.record_use(target, dummy_binding, false);
         }
     }
 }
@@ -709,7 +709,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                 }
             } else if is_indeterminate {
                 // Consider erroneous imports used to avoid duplicate diagnostics.
-                self.r.used_imports.insert((import.id, TypeNS));
+                self.r.used_imports.insert(import.id);
                 let path = import_path_to_string(
                     &import.module_path.iter().map(|seg| seg.ident).collect::<Vec<_>>(),
                     &import.kind,
@@ -902,7 +902,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
         import.vis.set(orig_vis);
         if let PathResult::Failed { .. } | PathResult::NonModule(..) = path_res {
             // Consider erroneous imports used to avoid duplicate diagnostics.
-            self.r.used_imports.insert((import.id, TypeNS));
+            self.r.used_imports.insert(import.id);
         }
         let module = match path_res {
             PathResult::Module(module) => {
@@ -1043,7 +1043,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
                                 {
                                     this.record_use(
                                         ident,
-                                        ns,
                                         target_binding,
                                         import.module_path.is_empty(),
                                     );

compiler/rustc_resolve/src/late.rs

@@ -1738,7 +1738,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
                 // whether they can be shadowed by fresh bindings or not, so force an error.
                 // issues/33118#issuecomment-233962221 (see below) still applies here,
                 // but we have to ignore it for backward compatibility.
-                self.r.record_use(ident, ValueNS, binding, false);
+                self.r.record_use(ident, binding, false);
                 return None;
             }
             LexicalScopeBinding::Item(binding) => (binding.res(), Some(binding)),
@@ -1753,7 +1753,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
             ) if is_syntactic_ambiguity => {
                 // Disambiguate in favor of a unit struct/variant or constant pattern.
                 if let Some(binding) = binding {
-                    self.r.record_use(ident, ValueNS, binding, false);
+                    self.r.record_use(ident, binding, false);
                 }
                 Some(res)
             }

compiler/rustc_resolve/src/lib.rs

@@ -942,7 +942,7 @@ pub struct Resolver<'a> {
     glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
     /// Visibilities in "lowered" form, for all entities that have them.
     visibilities: FxHashMap<LocalDefId, ty::Visibility>,
-    used_imports: FxHashSet<(NodeId, Namespace)>,
+    used_imports: FxHashSet<NodeId>,
     maybe_unused_trait_imports: FxHashSet<LocalDefId>,
     maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
 
@@ -1656,7 +1656,6 @@ impl<'a> Resolver<'a> {
     fn record_use(
         &mut self,
         ident: Ident,
-        ns: Namespace,
         used_binding: &'a NameBinding<'a>,
         is_lexical_scope: bool,
     ) {
@@ -1684,9 +1683,9 @@ impl<'a> Resolver<'a> {
             }
             used.set(true);
             import.used.set(true);
-            self.used_imports.insert((import.id, ns));
+            self.used_imports.insert(import.id);
             self.add_to_glob_map(&import, ident);
-            self.record_use(ident, ns, binding, false);
+            self.record_use(ident, binding, false);
         }
     }
 
@@ -3241,7 +3240,7 @@ impl<'a> Resolver<'a> {
         self.extern_prelude.get(&ident.normalize_to_macros_2_0()).cloned().and_then(|entry| {
             if let Some(binding) = entry.extern_crate_item {
                 if !speculative && entry.introduced_by_item {
-                    self.record_use(ident, TypeNS, binding, false);
+                    self.record_use(ident, binding, false);
                 }
                 Some(binding)
             } else {
@@ -3428,7 +3427,7 @@ impl<'a> Resolver<'a> {
         let is_import = name_binding.is_import();
         let span = name_binding.span;
         if let Res::Def(DefKind::Fn, _) = res {
-            self.record_use(ident, ValueNS, name_binding, false);
+            self.record_use(ident, name_binding, false);
         }
         self.main_def = Some(MainDefinition { res, is_import, span });
     }

compiler/rustc_resolve/src/macros.rs

@@ -1090,7 +1090,7 @@ impl<'a> Resolver<'a> {
             ) {
                 Ok(binding) => {
                     let initial_res = initial_binding.map(|initial_binding| {
-                        self.record_use(ident, MacroNS, initial_binding, false);
+                        self.record_use(ident, initial_binding, false);
                         initial_binding.res()
                     });
                     let res = binding.res();

compiler/rustc_target/src/spec/aarch64_apple_ios.rs

@@ -2,8 +2,15 @@ use super::apple_sdk_base::{opts, Arch};
 use crate::spec::{FramePointer, Target, TargetOptions};
 
 pub fn target() -> Target {
+    // Clang automatically chooses a more specific target based on
+    // IPHONEOS_DEPLOYMENT_TARGET.
+    // This is required for the target to pick the right
+    // MACH-O commands, so we do too.
+    let arch = "arm64";
+    let llvm_target = super::apple_base::ios_llvm_target(arch);
+
     Target {
-        llvm_target: "arm64-apple-ios".to_string(),
+        llvm_target,
         pointer_width: 64,
         data_layout: "e-m:o-i64:64-i128:128-n32:64-S128".to_string(),
         arch: "aarch64".to_string(),

compiler/rustc_target/src/spec/apple_base.rs

@@ -91,6 +91,11 @@ fn ios_deployment_target() -> (u32, u32) {
     deployment_target("IPHONEOS_DEPLOYMENT_TARGET").unwrap_or((7, 0))
 }
 
+pub fn ios_llvm_target(arch: &str) -> String {
+    let (major, minor) = ios_deployment_target();
+    format!("{}-apple-ios{}.{}.0", arch, major, minor)
+}
+
 pub fn ios_sim_llvm_target(arch: &str) -> String {
     let (major, minor) = ios_deployment_target();
     format!("{}-apple-ios{}.{}.0-simulator", arch, major, minor)

compiler/rustc_typeck/src/check/check.rs

@@ -1413,15 +1413,17 @@ fn check_enum<'tcx>(
                 Some(ref expr) => tcx.hir().span(expr.hir_id),
                 None => v.span,
             };
+            let display_discr = display_discriminant_value(tcx, v, discr.val);
+            let display_discr_i = display_discriminant_value(tcx, variant_i, disr_vals[i].val);
             struct_span_err!(
                 tcx.sess,
                 span,
                 E0081,
                 "discriminant value `{}` already exists",
-                disr_vals[i]
+                discr.val,
             )
-            .span_label(i_span, format!("first use of `{}`", disr_vals[i]))
-            .span_label(span, format!("enum already has `{}`", disr_vals[i]))
+            .span_label(i_span, format!("first use of {}", display_discr_i))
+            .span_label(span, format!("enum already has {}", display_discr))
             .emit();
         }
         disr_vals.push(discr);
@@ -1431,6 +1433,25 @@ fn check_enum<'tcx>(
     check_transparent(tcx, sp, def);
 }
 
+/// Format an enum discriminant value for use in a diagnostic message.
+fn display_discriminant_value<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    variant: &hir::Variant<'_>,
+    evaluated: u128,
+) -> String {
+    if let Some(expr) = &variant.disr_expr {
+        let body = &tcx.hir().body(expr.body).value;
+        if let hir::ExprKind::Lit(lit) = &body.kind {
+            if let rustc_ast::LitKind::Int(lit_value, _int_kind) = &lit.node {
+                if evaluated != *lit_value {
+                    return format!("`{}` (overflowed from `{}`)", evaluated, lit_value);
+                }
+            }
+        }
+    }
+    format!("`{}`", evaluated)
+}
+
 pub(super) fn check_type_params_are_used<'tcx>(
     tcx: TyCtxt<'tcx>,
     generics: &ty::Generics,

compiler/rustc_typeck/src/check/compare_method.rs

@@ -708,11 +708,7 @@ fn compare_number_of_method_arguments<'tcx>(
                         Some(if pos == 0 {
                             arg.span
                         } else {
-                            Span::new(
-                                trait_m_sig.decl.inputs[0].span.lo(),
-                                arg.span.hi(),
-                                arg.span.ctxt(),
-                            )
+                            arg.span.with_lo(trait_m_sig.decl.inputs[0].span.lo())
                         })
                     } else {
                         trait_item_span
@@ -731,11 +727,7 @@ fn compare_number_of_method_arguments<'tcx>(
                     if pos == 0 {
                         arg.span
                     } else {
-                        Span::new(
-                            impl_m_sig.decl.inputs[0].span.lo(),
-                            arg.span.hi(),
-                            arg.span.ctxt(),
-                        )
+                        arg.span.with_lo(impl_m_sig.decl.inputs[0].span.lo())
                     }
                 } else {
                     impl_m_span

src/test/ui/drop-bounds/drop-bounds.stderr

@@ -1,4 +1,4 @@
-error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
+error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped
   --> $DIR/drop-bounds.rs:2:11
    |
 LL | fn foo<T: Drop>() {}
@@ -10,37 +10,37 @@ note: the lint level is defined here
 LL | #![deny(drop_bounds)]
    |         ^^^^^^^^^^^
 
-error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
+error: bounds on `U: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped
   --> $DIR/drop-bounds.rs:5:8
    |
 LL |     U: Drop,
    |        ^^^^
 
-error: bounds on `impl Drop: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
+error: bounds on `impl Drop: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped
   --> $DIR/drop-bounds.rs:8:17
    |
 LL | fn baz(_x: impl Drop) {}
    |                 ^^^^
 
-error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
+error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped
   --> $DIR/drop-bounds.rs:9:15
    |
 LL | struct Foo<T: Drop> {
    |               ^^^^
 
-error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
+error: bounds on `U: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped
   --> $DIR/drop-bounds.rs:12:24
    |
 LL | struct Bar<U> where U: Drop {
    |                        ^^^^
 
-error: bounds on `Self: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
+error: bounds on `Self: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped
   --> $DIR/drop-bounds.rs:15:12
    |
 LL | trait Baz: Drop {
    |            ^^^^
 
-error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
+error: bounds on `T: Drop` are most likely incorrect, consider instead using `std::mem::needs_drop` to detect whether a type can be trivially dropped
   --> $DIR/drop-bounds.rs:17:9
    |
 LL | impl<T: Drop> Baz for T {