Skip to content

Commit

Permalink
Auto merge of #38271 - jseyfried:rfc_1560_warning_cycle, r=nrc
Browse files Browse the repository at this point in the history
resolve: change most backwards incompatible ambiguity errors to `legacy_imports` warnings

Fixes #38176.
r? @nrc or @nikomatsakis
  • Loading branch information
bors committed Dec 20, 2016
2 parents 94ae2a2 + cfabce2 commit 3038f30
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 28 deletions.
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ declare_lint! {
not named `mod.rs`"
}

declare_lint! {
pub LEGACY_IMPORTS,
Warn,
"detects names that resolve to ambiguous glob imports with RFC 1560"
}

declare_lint! {
pub DEPRECATED,
Warn,
Expand Down Expand Up @@ -257,6 +263,7 @@ impl LintPass for HardwiredLints {
PATTERNS_IN_FNS_WITHOUT_BODY,
EXTRA_REQUIREMENT_IN_IMPL,
LEGACY_DIRECTORY_OWNERSHIP,
LEGACY_IMPORTS,
DEPRECATED
)
}
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
id: LintId::of(LEGACY_DIRECTORY_OWNERSHIP),
reference: "issue #37872 <https://github.com/rust-lang/rust/issues/37872>",
},
FutureIncompatibleInfo {
id: LintId::of(LEGACY_IMPORTS),
reference: "issue #38260 <https://github.com/rust-lang/rust/issues/38260>",
},
]);

// Register renamed and removed lints
Expand Down
56 changes: 38 additions & 18 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ use syntax::ast::{Item, ItemKind, ImplItem, ImplItemKind};
use syntax::ast::{Local, Mutability, Pat, PatKind, Path};
use syntax::ast::{PathSegment, PathParameters, QSelf, TraitItemKind, TraitRef, Ty, TyKind};

use syntax_pos::{Span, DUMMY_SP};
use syntax_pos::{Span, DUMMY_SP, MultiSpan};
use errors::DiagnosticBuilder;

use std::cell::{Cell, RefCell};
Expand Down Expand Up @@ -896,6 +896,7 @@ enum NameBindingKind<'a> {
Ambiguity {
b1: &'a NameBinding<'a>,
b2: &'a NameBinding<'a>,
legacy: bool,
}
}

Expand All @@ -907,13 +908,15 @@ struct AmbiguityError<'a> {
lexical: bool,
b1: &'a NameBinding<'a>,
b2: &'a NameBinding<'a>,
legacy: bool,
}

impl<'a> NameBinding<'a> {
fn module(&self) -> Option<Module<'a>> {
match self.kind {
NameBindingKind::Module(module) => Some(module),
NameBindingKind::Import { binding, .. } => binding.module(),
NameBindingKind::Ambiguity { legacy: true, b1, .. } => b1.module(),
_ => None,
}
}
Expand All @@ -923,6 +926,7 @@ impl<'a> NameBinding<'a> {
NameBindingKind::Def(def) => def,
NameBindingKind::Module(module) => module.def().unwrap(),
NameBindingKind::Import { binding, .. } => binding.def(),
NameBindingKind::Ambiguity { legacy: true, b1, .. } => b1.def(),
NameBindingKind::Ambiguity { .. } => Def::Err,
}
}
Expand Down Expand Up @@ -1349,11 +1353,14 @@ impl<'a> Resolver<'a> {
self.record_use(name, ns, binding, span)
}
NameBindingKind::Import { .. } => false,
NameBindingKind::Ambiguity { b1, b2 } => {
NameBindingKind::Ambiguity { b1, b2, legacy } => {
self.ambiguity_errors.push(AmbiguityError {
span: span, name: name, lexical: false, b1: b1, b2: b2,
span: span, name: name, lexical: false, b1: b1, b2: b2, legacy: legacy,
});
true
if legacy {
self.record_use(name, ns, b1, span);
}
!legacy
}
_ => false
}
Expand Down Expand Up @@ -3065,26 +3072,39 @@ impl<'a> Resolver<'a> {
self.report_shadowing_errors();
let mut reported_spans = FxHashSet();

for &AmbiguityError { span, name, b1, b2, lexical } in &self.ambiguity_errors {
for &AmbiguityError { span, name, b1, b2, lexical, legacy } in &self.ambiguity_errors {
if !reported_spans.insert(span) { continue }
let participle = |binding: &NameBinding| {
if binding.is_import() { "imported" } else { "defined" }
};
let msg1 = format!("`{}` could resolve to the name {} here", name, participle(b1));
let msg2 = format!("`{}` could also resolve to the name {} here", name, participle(b2));
self.session.struct_span_err(span, &format!("`{}` is ambiguous", name))
.span_note(b1.span, &msg1)
.span_note(b2.span, &msg2)
.note(&if !lexical && b1.is_glob_import() {
format!("consider adding an explicit import of `{}` to disambiguate", name)
} else if let Def::Macro(..) = b1.def() {
format!("macro-expanded {} do not shadow",
if b1.is_import() { "macro imports" } else { "macros" })
} else {
format!("macro-expanded {} do not shadow when used in a macro invocation path",
if b1.is_import() { "imports" } else { "items" })
})
.emit();
let note = if !lexical && b1.is_glob_import() {
format!("consider adding an explicit import of `{}` to disambiguate", name)
} else if let Def::Macro(..) = b1.def() {
format!("macro-expanded {} do not shadow",
if b1.is_import() { "macro imports" } else { "macros" })
} else {
format!("macro-expanded {} do not shadow when used in a macro invocation path",
if b1.is_import() { "imports" } else { "items" })
};
if legacy {
let id = match b2.kind {
NameBindingKind::Import { directive, .. } => directive.id,
_ => unreachable!(),
};
let mut span = MultiSpan::from_span(span);
span.push_span_label(b1.span, msg1);
span.push_span_label(b2.span, msg2);
let msg = format!("`{}` is ambiguous", name);
self.session.add_lint(lint::builtin::LEGACY_IMPORTS, id, span, msg);
} else {
self.session.struct_span_err(span, &format!("`{}` is ambiguous", name))
.span_note(b1.span, &msg1)
.span_note(b2.span, &msg2)
.note(&note)
.emit();
}
}

for &PrivacyError(span, name, binding) in &self.privacy_errors {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ impl<'a> Resolver<'a> {
Some(shadower) if shadower.def() != binding.def() => {
self.ambiguity_errors.push(AmbiguityError {
span: span, name: name, b1: shadower, b2: binding, lexical: true,
legacy: false,
});
return Ok(shadower);
}
Expand Down
41 changes: 31 additions & 10 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ impl<'a> Resolver<'a> {
binding.def() != shadowed_glob.def() {
self.ambiguity_errors.push(AmbiguityError {
span: span, name: name, lexical: false, b1: binding, b2: shadowed_glob,
legacy: false,
});
}
}
Expand Down Expand Up @@ -336,9 +337,9 @@ impl<'a> Resolver<'a> {
}

pub fn ambiguity(&mut self, b1: &'a NameBinding<'a>, b2: &'a NameBinding<'a>)
-> &'a NameBinding<'a> {
-> &'a NameBinding<'a> {
self.arenas.alloc_name_binding(NameBinding {
kind: NameBindingKind::Ambiguity { b1: b1, b2: b2 },
kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: false },
vis: if b1.vis.is_at_least(b2.vis, self) { b1.vis } else { b2.vis },
span: b1.span,
expansion: Mark::root(),
Expand Down Expand Up @@ -726,7 +727,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}

for (&(name, ns), resolution) in module.resolutions.borrow().iter() {
let resolution = resolution.borrow();
let resolution = &mut *resolution.borrow_mut();
let binding = match resolution.binding {
Some(binding) => binding,
None => continue,
Expand All @@ -743,14 +744,34 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}
}

if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind {
if ns == TypeNS && orig_binding.is_variant() &&
!orig_binding.vis.is_at_least(binding.vis, self) {
let msg = format!("variant `{}` is private, and cannot be reexported \
(error E0364), consider declaring its enum as `pub`",
name);
self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, binding.span, msg);
match binding.kind {
NameBindingKind::Import { binding: orig_binding, directive, .. } => {
if ns == TypeNS && orig_binding.is_variant() &&
!orig_binding.vis.is_at_least(binding.vis, self) {
let msg = format!("variant `{}` is private, and cannot be reexported \
(error E0364), consider declaring its enum as `pub`",
name);
self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, binding.span, msg);
}
}
NameBindingKind::Ambiguity { b1, b2, .. }
if b1.is_glob_import() && b2.is_glob_import() => {
let (orig_b1, orig_b2) = match (&b1.kind, &b2.kind) {
(&NameBindingKind::Import { binding: b1, .. },
&NameBindingKind::Import { binding: b2, .. }) => (b1, b2),
_ => continue,
};
let (b1, b2) = match (orig_b1.vis, orig_b2.vis) {
(ty::Visibility::Public, ty::Visibility::Public) => continue,
(ty::Visibility::Public, _) => (b1, b2),
(_, ty::Visibility::Public) => (b2, b1),
_ => continue,
};
resolution.binding = Some(self.arenas.alloc_name_binding(NameBinding {
kind: NameBindingKind::Ambiguity { b1: b1, b2: b2, legacy: true }, ..*b1
}));
}
_ => {}
}
}

Expand Down
30 changes: 30 additions & 0 deletions src/test/compile-fail/imports/rfc-1560-warning-cycle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(rustc_attrs)]
#![allow(unused)]

pub struct Foo;

mod bar {
struct Foo;

mod baz {
use *; //~ NOTE `Foo` could resolve to the name imported here
use bar::*; //~ NOTE `Foo` could also resolve to the name imported here
fn f(_: Foo) {}
//~^ WARN `Foo` is ambiguous
//~| WARN hard error in a future release
//~| NOTE see issue #38260
}
}

#[rustc_error]
fn main() {} //~ ERROR compilation successful

0 comments on commit 3038f30

Please sign in to comment.