Skip to content

Commit af14075

Browse files
borsbors
committed
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
2 parents 91f9806 + 3e8e8d2 commit af14075

File tree

24 files changed

+180
-66
lines changed

24 files changed

+180
-66
lines changed

‎compiler/rustc_lint/src/traits.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,23 +18,27 @@ declare_lint! {
1818
///
1919
/// ### Explanation
2020
///
21-
/// `Drop` bounds do not really accomplish anything. A type may have
22-
/// compiler-generated drop glue without implementing the `Drop` trait
23-
/// itself. The `Drop` trait also only has one method, `Drop::drop`, and
24-
/// that function is by fiat not callable in user code. So there is really
25-
/// no use case for using `Drop` in trait bounds.
21+
/// A generic trait bound of the form `T: Drop` is most likely misleading
22+
/// and not what the programmer intended (they probably should have used
23+
/// `std::mem::needs_drop` instead).
2624
///
27-
/// The most likely use case of a drop bound is to distinguish between
28-
/// types that have destructors and types that don't. Combined with
29-
/// specialization, a naive coder would write an implementation that
30-
/// assumed a type could be trivially dropped, then write a specialization
31-
/// for `T: Drop` that actually calls the destructor. Except that doing so
32-
/// is not correct; String, for example, doesn't actually implement Drop,
33-
/// but because String contains a Vec, assuming it can be trivially dropped
34-
/// will leak memory.
25+
/// `Drop` bounds do not actually indicate whether a type can be trivially
26+
/// dropped or not, because a composite type containing `Drop` types does
27+
/// not necessarily implement `Drop` itself. Naïvely, one might be tempted
28+
/// to write an implementation that assumes that a type can be trivially
29+
/// dropped while also supplying a specialization for `T: Drop` that
30+
/// actually calls the destructor. However, this breaks down e.g. when `T`
31+
/// is `String`, which does not implement `Drop` itself but contains a
32+
/// `Vec`, which does implement `Drop`, so assuming `T` can be trivially
33+
/// dropped would lead to a memory leak here.
34+
///
35+
/// Furthermore, the `Drop` trait only contains one method, `Drop::drop`,
36+
/// which may not be called explicitly in user code (`E0040`), so there is
37+
/// really no use case for using `Drop` in trait bounds, save perhaps for
38+
/// some obscure corner cases, which can use `#[allow(drop_bounds)]`.
3539
pub DROP_BOUNDS,
3640
Warn,
37-
"bounds of the form `T: Drop` are useless"
41+
"bounds of the form `T: Drop` are most likely incorrect"
3842
}
3943

4044
declare_lint! {
@@ -102,8 +106,8 @@ impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints {
102106
None => return,
103107
};
104108
let msg = format!(
105-
"bounds on `{}` are useless, consider instead \
106-
using `{}` to detect if a type has a destructor",
109+
"bounds on `{}` are most likely incorrect, consider instead \
110+
using `{}` to detect whether a type can be trivially dropped",
107111
predicate,
108112
cx.tcx.def_path_str(needs_drop)
109113
);

‎compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,9 +922,17 @@ LLVMRustOptimizeWithNewPassManager(
922922
MPM.addPass(RequireAnalysisPass<ASanGlobalsMetadataAnalysis, Module>());
923923
MPM.addPass(ModuleAddressSanitizerPass(
924924
/*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover));
925+
#if LLVM_VERSION_GE(14, 0)
926+
AddressSanitizerOptions opts(/*CompileKernel=*/false,
927+
SanitizerOptions->SanitizeAddressRecover,
928+
/*UseAfterScope=*/true,
929+
AsanDetectStackUseAfterReturnMode::Runtime);
930+
MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass(opts)));
931+
#else
925932
MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass(
926933
/*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover,
927934
/*UseAfterScope=*/true)));
935+
#endif
928936
}
929937
);
930938
#else
@@ -952,8 +960,15 @@ LLVMRustOptimizeWithNewPassManager(
952960
#if LLVM_VERSION_GE(11, 0)
953961
OptimizerLastEPCallbacks.push_back(
954962
[SanitizerOptions](ModulePassManager &MPM, OptimizationLevel Level) {
963+
#if LLVM_VERSION_GE(14, 0)
964+
HWAddressSanitizerOptions opts(
965+
/*CompileKernel=*/false, SanitizerOptions->SanitizeHWAddressRecover,
966+
/*DisableOptimization=*/false);
967+
MPM.addPass(HWAddressSanitizerPass(opts));
968+
#else
955969
MPM.addPass(HWAddressSanitizerPass(
956970
/*CompileKernel=*/false, SanitizerOptions->SanitizeHWAddressRecover));
971+
#endif
957972
}
958973
);
959974
#else

‎compiler/rustc_middle/src/middle/region.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ impl Scope {
189189
// To avoid issues with macro-generated spans, the span
190190
// of the statement must be nested in that of the block.
191191
if span.lo() <= stmt_span.lo() && stmt_span.lo() <= span.hi() {
192-
return Span::new(stmt_span.lo(), span.hi(), span.ctxt());
192+
return span.with_lo(stmt_span.lo());
193193
}
194194
}
195195
}

‎compiler/rustc_resolve/src/check_unused.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,7 @@ impl<'a, 'b> UnusedImportCheckVisitor<'a, 'b> {
6363
// We have information about whether `use` (import) items are actually
6464
// used now. If an import is not used at all, we signal a lint error.
6565
fn check_import(&mut self, id: ast::NodeId) {
66-
let mut used = false;
67-
self.r.per_ns(|this, ns| used |= this.used_imports.contains(&(id, ns)));
66+
let used = self.r.used_imports.contains(&id);
6867
let def_id = self.r.local_def_id(id);
6968
if !used {
7069
if self.r.maybe_unused_trait_imports.contains(&def_id) {

‎compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -950,9 +950,7 @@ impl<'a> Resolver<'a> {
950950
self.add_typo_suggestion(err, suggestion, ident.span);
951951

952952
let import_suggestions =
953-
self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, |res| {
954-
matches!(res, Res::Def(DefKind::Macro(MacroKind::Bang), _))
955-
});
953+
self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
956954
show_candidates(err, None, &import_suggestions, false, true);
957955

958956
if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {

‎compiler/rustc_resolve/src/imports.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ impl<'a> Resolver<'a> {
303303
if self.last_import_segment && check_usable(self, binding).is_err() {
304304
Err((Determined, Weak::No))
305305
} else {
306-
self.record_use(ident, ns, binding, restricted_shadowing);
306+
self.record_use(ident, binding, restricted_shadowing);
307307

308308
if let Some(shadowed_glob) = resolution.shadowed_glob {
309309
// Forbid expanded shadowing to avoid time travel.
@@ -609,9 +609,9 @@ impl<'a> Resolver<'a> {
609609
self.per_ns(|this, ns| {
610610
let key = this.new_key(target, ns);
611611
let _ = this.try_define(import.parent_scope.module, key, dummy_binding);
612-
// Consider erroneous imports used to avoid duplicate diagnostics.
613-
this.record_use(target, ns, dummy_binding, false);
614612
});
613+
// Consider erroneous imports used to avoid duplicate diagnostics.
614+
self.record_use(target, dummy_binding, false);
615615
}
616616
}
617617
}
@@ -709,7 +709,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
709709
}
710710
} else if is_indeterminate {
711711
// Consider erroneous imports used to avoid duplicate diagnostics.
712-
self.r.used_imports.insert((import.id, TypeNS));
712+
self.r.used_imports.insert(import.id);
713713
let path = import_path_to_string(
714714
&import.module_path.iter().map(|seg| seg.ident).collect::<Vec<_>>(),
715715
&import.kind,
@@ -902,7 +902,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
902902
import.vis.set(orig_vis);
903903
if let PathResult::Failed { .. } | PathResult::NonModule(..) = path_res {
904904
// Consider erroneous imports used to avoid duplicate diagnostics.
905-
self.r.used_imports.insert((import.id, TypeNS));
905+
self.r.used_imports.insert(import.id);
906906
}
907907
let module = match path_res {
908908
PathResult::Module(module) => {
@@ -1043,7 +1043,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
10431043
{
10441044
this.record_use(
10451045
ident,
1046-
ns,
10471046
target_binding,
10481047
import.module_path.is_empty(),
10491048
);

‎compiler/rustc_resolve/src/late.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,7 +1738,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
17381738
// whether they can be shadowed by fresh bindings or not, so force an error.
17391739
// issues/33118#issuecomment-233962221 (see below) still applies here,
17401740
// but we have to ignore it for backward compatibility.
1741-
self.r.record_use(ident, ValueNS, binding, false);
1741+
self.r.record_use(ident, binding, false);
17421742
return None;
17431743
}
17441744
LexicalScopeBinding::Item(binding) => (binding.res(), Some(binding)),
@@ -1753,7 +1753,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
17531753
) if is_syntactic_ambiguity => {
17541754
// Disambiguate in favor of a unit struct/variant or constant pattern.
17551755
if let Some(binding) = binding {
1756-
self.r.record_use(ident, ValueNS, binding, false);
1756+
self.r.record_use(ident, binding, false);
17571757
}
17581758
Some(res)
17591759
}

‎compiler/rustc_resolve/src/lib.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@ pub struct Resolver<'a> {
942942
glob_map: FxHashMap<LocalDefId, FxHashSet<Symbol>>,
943943
/// Visibilities in "lowered" form, for all entities that have them.
944944
visibilities: FxHashMap<LocalDefId, ty::Visibility>,
945-
used_imports: FxHashSet<(NodeId, Namespace)>,
945+
used_imports: FxHashSet<NodeId>,
946946
maybe_unused_trait_imports: FxHashSet<LocalDefId>,
947947
maybe_unused_extern_crates: Vec<(LocalDefId, Span)>,
948948

@@ -1656,7 +1656,6 @@ impl<'a> Resolver<'a> {
16561656
fn record_use(
16571657
&mut self,
16581658
ident: Ident,
1659-
ns: Namespace,
16601659
used_binding: &'a NameBinding<'a>,
16611660
is_lexical_scope: bool,
16621661
) {
@@ -1684,9 +1683,9 @@ impl<'a> Resolver<'a> {
16841683
}
16851684
used.set(true);
16861685
import.used.set(true);
1687-
self.used_imports.insert((import.id, ns));
1686+
self.used_imports.insert(import.id);
16881687
self.add_to_glob_map(&import, ident);
1689-
self.record_use(ident, ns, binding, false);
1688+
self.record_use(ident, binding, false);
16901689
}
16911690
}
16921691

@@ -3241,7 +3240,7 @@ impl<'a> Resolver<'a> {
32413240
self.extern_prelude.get(&ident.normalize_to_macros_2_0()).cloned().and_then(|entry| {
32423241
if let Some(binding) = entry.extern_crate_item {
32433242
if !speculative && entry.introduced_by_item {
3244-
self.record_use(ident, TypeNS, binding, false);
3243+
self.record_use(ident, binding, false);
32453244
}
32463245
Some(binding)
32473246
} else {
@@ -3428,7 +3427,7 @@ impl<'a> Resolver<'a> {
34283427
let is_import = name_binding.is_import();
34293428
let span = name_binding.span;
34303429
if let Res::Def(DefKind::Fn, _) = res {
3431-
self.record_use(ident, ValueNS, name_binding, false);
3430+
self.record_use(ident, name_binding, false);
34323431
}
34333432
self.main_def = Some(MainDefinition { res, is_import, span });
34343433
}

‎compiler/rustc_resolve/src/macros.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1090,7 +1090,7 @@ impl<'a> Resolver<'a> {
10901090
) {
10911091
Ok(binding) => {
10921092
let initial_res = initial_binding.map(|initial_binding| {
1093-
self.record_use(ident, MacroNS, initial_binding, false);
1093+
self.record_use(ident, initial_binding, false);
10941094
initial_binding.res()
10951095
});
10961096
let res = binding.res();

‎compiler/rustc_target/src/spec/aarch64_apple_ios.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,15 @@ use super::apple_sdk_base::{opts, Arch};
22
use crate::spec::{FramePointer, Target, TargetOptions};
33

44
pub fn target() -> Target {
5+
// Clang automatically chooses a more specific target based on
6+
// IPHONEOS_DEPLOYMENT_TARGET.
7+
// This is required for the target to pick the right
8+
// MACH-O commands, so we do too.
9+
let arch = "arm64";
10+
let llvm_target = super::apple_base::ios_llvm_target(arch);
11+
512
Target {
6-
llvm_target: "arm64-apple-ios".to_string(),
13+
llvm_target,
714
pointer_width: 64,
815
data_layout: "e-m:o-i64:64-i128:128-n32:64-S128".to_string(),
916
arch: "aarch64".to_string(),

‎compiler/rustc_target/src/spec/apple_base.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ fn ios_deployment_target() -> (u32, u32) {
9191
deployment_target("IPHONEOS_DEPLOYMENT_TARGET").unwrap_or((7, 0))
9292
}
9393

94+
pub fn ios_llvm_target(arch: &str) -> String {
95+
let (major, minor) = ios_deployment_target();
96+
format!("{}-apple-ios{}.{}.0", arch, major, minor)
97+
}
98+
9499
pub fn ios_sim_llvm_target(arch: &str) -> String {
95100
let (major, minor) = ios_deployment_target();
96101
format!("{}-apple-ios{}.{}.0-simulator", arch, major, minor)

‎compiler/rustc_typeck/src/check/check.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,15 +1413,17 @@ fn check_enum<'tcx>(
14131413
Some(ref expr) => tcx.hir().span(expr.hir_id),
14141414
None => v.span,
14151415
};
1416+
let display_discr = display_discriminant_value(tcx, v, discr.val);
1417+
let display_discr_i = display_discriminant_value(tcx, variant_i, disr_vals[i].val);
14161418
struct_span_err!(
14171419
tcx.sess,
14181420
span,
14191421
E0081,
14201422
"discriminant value `{}` already exists",
1421-
disr_vals[i]
1423+
discr.val,
14221424
)
1423-
.span_label(i_span, format!("first use of `{}`", disr_vals[i]))
1424-
.span_label(span, format!("enum already has `{}`", disr_vals[i]))
1425+
.span_label(i_span, format!("first use of {}", display_discr_i))
1426+
.span_label(span, format!("enum already has {}", display_discr))
14251427
.emit();
14261428
}
14271429
disr_vals.push(discr);
@@ -1431,6 +1433,25 @@ fn check_enum<'tcx>(
14311433
check_transparent(tcx, sp, def);
14321434
}
14331435

1436+
/// Format an enum discriminant value for use in a diagnostic message.
1437+
fn display_discriminant_value<'tcx>(
1438+
tcx: TyCtxt<'tcx>,
1439+
variant: &hir::Variant<'_>,
1440+
evaluated: u128,
1441+
) -> String {
1442+
if let Some(expr) = &variant.disr_expr {
1443+
let body = &tcx.hir().body(expr.body).value;
1444+
if let hir::ExprKind::Lit(lit) = &body.kind {
1445+
if let rustc_ast::LitKind::Int(lit_value, _int_kind) = &lit.node {
1446+
if evaluated != *lit_value {
1447+
return format!("`{}` (overflowed from `{}`)", evaluated, lit_value);
1448+
}
1449+
}
1450+
}
1451+
}
1452+
format!("`{}`", evaluated)
1453+
}
1454+
14341455
pub(super) fn check_type_params_are_used<'tcx>(
14351456
tcx: TyCtxt<'tcx>,
14361457
generics: &ty::Generics,

‎compiler/rustc_typeck/src/check/compare_method.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -708,11 +708,7 @@ fn compare_number_of_method_arguments<'tcx>(
708708
Some(if pos == 0 {
709709
arg.span
710710
} else {
711-
Span::new(
712-
trait_m_sig.decl.inputs[0].span.lo(),
713-
arg.span.hi(),
714-
arg.span.ctxt(),
715-
)
711+
arg.span.with_lo(trait_m_sig.decl.inputs[0].span.lo())
716712
})
717713
} else {
718714
trait_item_span
@@ -731,11 +727,7 @@ fn compare_number_of_method_arguments<'tcx>(
731727
if pos == 0 {
732728
arg.span
733729
} else {
734-
Span::new(
735-
impl_m_sig.decl.inputs[0].span.lo(),
736-
arg.span.hi(),
737-
arg.span.ctxt(),
738-
)
730+
arg.span.with_lo(impl_m_sig.decl.inputs[0].span.lo())
739731
}
740732
} else {
741733
impl_m_span

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
1+
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
22
--> $DIR/drop-bounds.rs:2:11
33
|
44
LL | fn foo<T: Drop>() {}
@@ -10,37 +10,37 @@ note: the lint level is defined here
1010
LL | #![deny(drop_bounds)]
1111
| ^^^^^^^^^^^
1212

13-
error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
13+
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
1414
--> $DIR/drop-bounds.rs:5:8
1515
|
1616
LL | U: Drop,
1717
| ^^^^
1818

19-
error: bounds on `impl Drop: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
19+
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
2020
--> $DIR/drop-bounds.rs:8:17
2121
|
2222
LL | fn baz(_x: impl Drop) {}
2323
| ^^^^
2424

25-
error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
25+
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
2626
--> $DIR/drop-bounds.rs:9:15
2727
|
2828
LL | struct Foo<T: Drop> {
2929
| ^^^^
3030

31-
error: bounds on `U: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
31+
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
3232
--> $DIR/drop-bounds.rs:12:24
3333
|
3434
LL | struct Bar<U> where U: Drop {
3535
| ^^^^
3636

37-
error: bounds on `Self: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
37+
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
3838
--> $DIR/drop-bounds.rs:15:12
3939
|
4040
LL | trait Baz: Drop {
4141
| ^^^^
4242

43-
error: bounds on `T: Drop` are useless, consider instead using `std::mem::needs_drop` to detect if a type has a destructor
43+
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
4444
--> $DIR/drop-bounds.rs:17:9
4545
|
4646
LL | impl<T: Drop> Baz for T {

0 commit comments

Comments
 (0)