From b36b2e89043fbaafb973999432306879e4348f29 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 8 Apr 2014 14:31:25 -0700 Subject: [PATCH 1/3] rustc: Disallow importing through use statements Resolve is currently erroneously allowing imports through private `use` statements in some circumstances, even across module boundaries. For example, this code compiles successfully today: use std::c_str; mod test { use c_str::CString; } This should not be allowed because it was explicitly decided that private `use` statements are purely bringing local names into scope, they are not participating further in name resolution. As a consequence of this patch, this code, while valid today, is now invalid: mod test { use std::c_str; unsafe fn foo() { ::test::c_str::CString::new(0 as *u8, false); } } While plausibly acceptable, I found it to be more consistent if private imports were only considered candidates to resolve the first component in a path, and no others. Closes #12612 --- src/libnative/io/file_win32.rs | 2 +- src/librustc/front/test.rs | 14 ++++++++------ src/librustc/middle/resolve.rs | 22 +++++++++++++++++----- src/test/auxiliary/issue-12612-1.rs | 13 +++++++++++++ src/test/auxiliary/issue-12612-2.rs | 11 +++++++++++ src/test/compile-fail/issue-12612.rs | 24 ++++++++++++++++++++++++ src/test/run-pass/issue-11881.rs | 10 ++++------ src/test/run-pass/issue-12612.rs | 23 +++++++++++++++++++++++ 8 files changed, 101 insertions(+), 18 deletions(-) create mode 100644 src/test/auxiliary/issue-12612-1.rs create mode 100644 src/test/auxiliary/issue-12612-2.rs create mode 100644 src/test/compile-fail/issue-12612.rs create mode 100644 src/test/run-pass/issue-12612.rs diff --git a/src/libnative/io/file_win32.rs b/src/libnative/io/file_win32.rs index 3e8ee55df94fc..56a4122f8f991 100644 --- a/src/libnative/io/file_win32.rs +++ b/src/libnative/io/file_win32.rs @@ -324,7 +324,7 @@ pub fn mkdir(p: &CString, _mode: io::FilePermission) -> IoResult<()> { } pub fn readdir(p: &CString) -> IoResult<~[Path]> { - use rt::global_heap::malloc_raw; + use std::rt::global_heap::malloc_raw; fn prune(root: &CString, dirs: ~[Path]) -> ~[Path] { let root = unsafe { CString::new(root.with_ref(|p| p), false) }; diff --git a/src/librustc/front/test.rs b/src/librustc/front/test.rs index 86d2e039505f1..dff3d8b03bcbf 100644 --- a/src/librustc/front/test.rs +++ b/src/librustc/front/test.rs @@ -297,20 +297,22 @@ mod __test { fn mk_std(cx: &TestCtxt) -> ast::ViewItem { let id_test = token::str_to_ident("test"); - let vi = if cx.is_test_crate { - ast::ViewItemUse( + let (vi, vis) = if cx.is_test_crate { + (ast::ViewItemUse( vec!(@nospan(ast::ViewPathSimple(id_test, path_node(vec!(id_test)), - ast::DUMMY_NODE_ID)))) + ast::DUMMY_NODE_ID)))), + ast::Public) } else { - ast::ViewItemExternCrate(id_test, + (ast::ViewItemExternCrate(id_test, with_version("test"), - ast::DUMMY_NODE_ID) + ast::DUMMY_NODE_ID), + ast::Inherited) }; ast::ViewItem { node: vi, attrs: Vec::new(), - vis: ast::Inherited, + vis: vis, span: DUMMY_SP } } diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 143b02f96d22f..df26f6db08a85 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -2286,10 +2286,12 @@ impl<'a> Resolver<'a> { } Some(child_name_bindings) => { if child_name_bindings.defined_in_namespace(ValueNS) { + debug!("(resolving single import) found value binding"); value_result = BoundResult(containing_module, *child_name_bindings); } if child_name_bindings.defined_in_namespace(TypeNS) { + debug!("(resolving single import) found type binding"); type_result = BoundResult(containing_module, *child_name_bindings); } @@ -2320,6 +2322,7 @@ impl<'a> Resolver<'a> { .borrow(); match import_resolutions.find(&source.name) { None => { + debug!("(resolving single import) no import"); // The containing module definitely doesn't have an // exported import with the name in question. We can // therefore accurately report that the names are @@ -2353,6 +2356,8 @@ impl<'a> Resolver<'a> { return UnboundResult; } Some(target) => { + debug!("(resolving single import) found \ + import in ns {:?}", namespace); let id = import_resolution.id(namespace); this.used_imports.insert((id, namespace)); return BoundResult(target.target_module, @@ -2396,6 +2401,8 @@ impl<'a> Resolver<'a> { .find_copy(&source.name) { None => {} // Continue. Some(module) => { + debug!("(resolving single import) found external \ + module"); let name_bindings = @Resolver::create_name_bindings_from_module( module); @@ -2669,7 +2676,8 @@ impl<'a> Resolver<'a> { match self.resolve_name_in_module(search_module, name, TypeNS, - name_search_type) { + name_search_type, + false) { Failed => { let segment_name = token::get_ident(name); let module_name = self.module_to_str(search_module); @@ -2977,7 +2985,8 @@ impl<'a> Resolver<'a> { match self.resolve_name_in_module(search_module, name, namespace, - PathSearch) { + PathSearch, + true) { Failed => { // Continue up the search chain. } @@ -3141,7 +3150,8 @@ impl<'a> Resolver<'a> { module_: @Module, name: Ident, namespace: Namespace, - name_search_type: NameSearchType) + name_search_type: NameSearchType, + allow_private_imports: bool) -> ResolveResult<(Target, bool)> { debug!("(resolving name in module) resolving `{}` in `{}`", token::get_ident(name), @@ -3172,7 +3182,9 @@ impl<'a> Resolver<'a> { // Check the list of resolved imports. match module_.import_resolutions.borrow().find(&name.name) { - Some(import_resolution) => { + Some(import_resolution) if allow_private_imports || + import_resolution.is_public.get() => { + if import_resolution.is_public.get() && import_resolution.outstanding_references.get() != 0 { debug!("(resolving name in module) import \ @@ -3193,7 +3205,7 @@ impl<'a> Resolver<'a> { } } } - None => {} // Continue. + Some(..) | None => {} // Continue. } // Finally, search through external children. diff --git a/src/test/auxiliary/issue-12612-1.rs b/src/test/auxiliary/issue-12612-1.rs new file mode 100644 index 0000000000000..a0234c1185a97 --- /dev/null +++ b/src/test/auxiliary/issue-12612-1.rs @@ -0,0 +1,13 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub mod bar { + pub fn foo() {} +} diff --git a/src/test/auxiliary/issue-12612-2.rs b/src/test/auxiliary/issue-12612-2.rs new file mode 100644 index 0000000000000..b4ae4374b2e58 --- /dev/null +++ b/src/test/auxiliary/issue-12612-2.rs @@ -0,0 +1,11 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub fn baz() {} diff --git a/src/test/compile-fail/issue-12612.rs b/src/test/compile-fail/issue-12612.rs new file mode 100644 index 0000000000000..9d6eb42567893 --- /dev/null +++ b/src/test/compile-fail/issue-12612.rs @@ -0,0 +1,24 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:issue-12612-1.rs + +extern crate foo = "issue-12612-1"; + +use foo::bar; + +mod test { + use bar::foo; + //~^ ERROR: unresolved import + //~^^ ERROR: failed to resolve import +} + +fn main() {} + diff --git a/src/test/run-pass/issue-11881.rs b/src/test/run-pass/issue-11881.rs index 2bf846fe3419c..7e51c6ad2aebb 100644 --- a/src/test/run-pass/issue-11881.rs +++ b/src/test/run-pass/issue-11881.rs @@ -8,13 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -extern crate ser = "serialize"; +extern crate serialize; -use serialize = self::ser; - //necessary for deriving(Encodable) -use ser::{Encodable, Encoder}; -use ser::json; -use ser::ebml::writer; +use serialize::{Encodable, Encoder}; +use serialize::json; +use serialize::ebml::writer; use std::io::MemWriter; use std::str::from_utf8_owned; diff --git a/src/test/run-pass/issue-12612.rs b/src/test/run-pass/issue-12612.rs new file mode 100644 index 0000000000000..fcb658036b6b1 --- /dev/null +++ b/src/test/run-pass/issue-12612.rs @@ -0,0 +1,23 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// aux-build:issue-12612-1.rs +// aux-build:issue-12612-2.rs + +extern crate foo = "issue-12612-1"; +extern crate bar = "issue-12612-2"; + +use foo::bar; + +mod test { + use bar::baz; +} + +fn main() {} From 015a9c58b2925a8ee1e31aede71740b9d5a5ec8f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 8 Apr 2014 15:03:29 -0700 Subject: [PATCH 2/3] rustc: Don't succeed on shadowed nonexistent import Previously resolve was checking the "import resolution" for whether an import had succeeded or not, but this was the same structure filled in by a previous import if a name is shadowed. Instead, this alters resolve to consult the local resolve state (as opposed to the shared one) to test whether an import succeeded or not. Closes #13404 --- src/liblibc/lib.rs | 2 +- src/librustc/middle/resolve.rs | 9 +++++++-- src/test/compile-fail/issue-13404.rs | 21 +++++++++++++++++++++ 3 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 src/test/compile-fail/issue-13404.rs diff --git a/src/liblibc/lib.rs b/src/liblibc/lib.rs index 6b519fa7b83b7..18e815e9b7c03 100644 --- a/src/liblibc/lib.rs +++ b/src/liblibc/lib.rs @@ -146,7 +146,7 @@ pub use types::os::arch::c95::{c_ushort, clock_t, ptrdiff_t}; pub use types::os::arch::c95::{size_t, time_t}; pub use types::os::arch::c99::{c_longlong, c_ulonglong, intptr_t}; pub use types::os::arch::c99::{uintptr_t}; -pub use types::os::arch::posix88::{dev_t, dirent_t, ino_t, mode_t}; +pub use types::os::arch::posix88::{dev_t, ino_t, mode_t}; pub use types::os::arch::posix88::{off_t, pid_t, ssize_t}; pub use consts::os::c95::{_IOFBF, _IOLBF, _IONBF, BUFSIZ, EOF}; diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index df26f6db08a85..08b36bde3650b 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -144,6 +144,12 @@ impl NamespaceResult { _ => false } } + fn is_unbound(&self) -> bool { + match *self { + UnboundResult => true, + _ => false + } + } } enum NameDefinition { @@ -2449,8 +2455,7 @@ impl<'a> Resolver<'a> { } } - if import_resolution.value_target.borrow().is_none() && - import_resolution.type_target.borrow().is_none() { + if value_result.is_unbound() && type_result.is_unbound() { let msg = format!("unresolved import: there is no \ `{}` in `{}`", token::get_ident(source), diff --git a/src/test/compile-fail/issue-13404.rs b/src/test/compile-fail/issue-13404.rs new file mode 100644 index 0000000000000..2ce502ee8dbb3 --- /dev/null +++ b/src/test/compile-fail/issue-13404.rs @@ -0,0 +1,21 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use a::f; +use b::f; +//~^ ERROR: unresolved import +//~^^ ERROR: failed to resolve import + +mod a { pub fn f() {} } +mod b { } + +fn main() { + f(); +} From 1100f0ad1783bd87d444406ab394ca405f0ab9a9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 8 Apr 2014 15:10:41 -0700 Subject: [PATCH 3/3] rustc: Don't allow priv use to shadow pub use Previously, a private use statement would shadow a public use statement, all of a sudden publicly exporting the privately used item. The correct behavior here is to only shadow the use for the module in question, but for now it just reverts the entire name to private so the pub use doesn't have much effect. The behavior isn't exactly what we want, but this no longer has backwards compatibility hazards. --- src/librustc/middle/resolve.rs | 1 + src/libstd/io/test.rs | 1 - .../resolve-priv-shadowing-pub.rs | 33 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/resolve-priv-shadowing-pub.rs diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 08b36bde3650b..202eddacb03c4 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -1982,6 +1982,7 @@ impl<'a> Resolver<'a> { // the source of this name is different now resolution.type_id.set(id); resolution.value_id.set(id); + resolution.is_public.set(is_public); } None => { debug!("(building import directive) creating new"); diff --git a/src/libstd/io/test.rs b/src/libstd/io/test.rs index 14b9b5c1e061b..dd874fecc52a5 100644 --- a/src/libstd/io/test.rs +++ b/src/libstd/io/test.rs @@ -39,7 +39,6 @@ macro_rules! iotest ( use io::process::*; use unstable::running_on_valgrind; use str; - use util; fn f() $b diff --git a/src/test/compile-fail/resolve-priv-shadowing-pub.rs b/src/test/compile-fail/resolve-priv-shadowing-pub.rs new file mode 100644 index 0000000000000..0830722f96929 --- /dev/null +++ b/src/test/compile-fail/resolve-priv-shadowing-pub.rs @@ -0,0 +1,33 @@ +// Copyright 2014 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +mod a { + pub fn foobar() -> int { 1 } +} + +mod b { + pub fn foobar() -> int { 2 } +} + +mod c { + // Technically the second use shadows the first, but in theory it should + // only be shadowed for this module. The implementation of resolve currently + // doesn't implement this, so this test is ensuring that using "c::foobar" + // is *not* getting b::foobar. Today it's an error, but perhaps one day it + // can correctly get a::foobar instead. + pub use a::foobar; + use b::foobar; +} + +fn main() { + assert_eq!(c::foobar(), 1); + //~^ ERROR: unresolved name `c::foobar` +} +