Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 7 pull requests #88240

Merged
merged 15 commits into from
Aug 22, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions compiler/rustc_lint/src/traits.rs
Original file line number Diff line number Diff line change
@@ -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)
);
15 changes: 15 additions & 0 deletions compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/middle/region.rs
Original file line number Diff line number Diff line change
@@ -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());
}
}
}
3 changes: 1 addition & 2 deletions compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
@@ -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) {
4 changes: 1 addition & 3 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -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) {
11 changes: 5 additions & 6 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
@@ -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(),
);
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
@@ -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)
}
11 changes: 5 additions & 6 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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 });
}
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
@@ -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();
9 changes: 8 additions & 1 deletion compiler/rustc_target/src/spec/aarch64_apple_ios.rs
Original file line number Diff line number Diff line change
@@ -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(),
5 changes: 5 additions & 0 deletions compiler/rustc_target/src/spec/apple_base.rs
Original file line number Diff line number Diff line change
@@ -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)
27 changes: 24 additions & 3 deletions compiler/rustc_typeck/src/check/check.rs
Original file line number Diff line number Diff line change
@@ -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,
12 changes: 2 additions & 10 deletions compiler/rustc_typeck/src/check/compare_method.rs
Original file line number Diff line number Diff line change
@@ -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
14 changes: 7 additions & 7 deletions src/test/ui/drop-bounds/drop-bounds.stderr
Original file line number Diff line number Diff line change
@@ -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 {
Loading