Skip to content

Distinguish argument from local variable #29327

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

Merged
merged 2 commits into from
Oct 27, 2015
Merged
Show file tree
Hide file tree
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
17 changes: 0 additions & 17 deletions src/librustc/front/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,6 @@ impl<'ast> NodeCollector<'ast> {
let entry = MapEntry::from_node(self.parent_node, node);
self.insert_entry(id, entry);
}

fn visit_fn_decl(&mut self, decl: &'ast FnDecl) {
for a in &decl.inputs {
self.insert(a.id, NodeArg(&*a.pat));
}
}
}

impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
Expand Down Expand Up @@ -295,20 +289,9 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
fn visit_fn(&mut self, fk: visit::FnKind<'ast>, fd: &'ast FnDecl,
b: &'ast Block, s: Span, id: NodeId) {
assert_eq!(self.parent_node, id);
self.visit_fn_decl(fd);
visit::walk_fn(self, fk, fd, b, s);
}

fn visit_ty(&mut self, ty: &'ast Ty) {
match ty.node {
TyBareFn(ref fd) => {
self.visit_fn_decl(&*fd.decl);
}
_ => {}
}
visit::walk_ty(self, ty);
}

fn visit_block(&mut self, block: &'ast Block) {
self.insert(block.id, NodeBlock(block));
let parent_node = self.parent_node;
Expand Down
32 changes: 22 additions & 10 deletions src/librustc/front/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ pub enum Node<'ast> {
NodeVariant(&'ast Variant),
NodeExpr(&'ast Expr),
NodeStmt(&'ast Stmt),
NodeArg(&'ast Pat),
NodeLocal(&'ast Pat),
NodePat(&'ast Pat),
NodeBlock(&'ast Block),
Expand All @@ -145,7 +144,6 @@ pub enum MapEntry<'ast> {
EntryVariant(NodeId, &'ast Variant),
EntryExpr(NodeId, &'ast Expr),
EntryStmt(NodeId, &'ast Stmt),
EntryArg(NodeId, &'ast Pat),
EntryLocal(NodeId, &'ast Pat),
EntryPat(NodeId, &'ast Pat),
EntryBlock(NodeId, &'ast Block),
Expand Down Expand Up @@ -180,7 +178,6 @@ impl<'ast> MapEntry<'ast> {
NodeVariant(n) => EntryVariant(p, n),
NodeExpr(n) => EntryExpr(p, n),
NodeStmt(n) => EntryStmt(p, n),
NodeArg(n) => EntryArg(p, n),
NodeLocal(n) => EntryLocal(p, n),
NodePat(n) => EntryPat(p, n),
NodeBlock(n) => EntryBlock(p, n),
Expand All @@ -199,7 +196,6 @@ impl<'ast> MapEntry<'ast> {
EntryVariant(id, _) => id,
EntryExpr(id, _) => id,
EntryStmt(id, _) => id,
EntryArg(id, _) => id,
EntryLocal(id, _) => id,
EntryPat(id, _) => id,
EntryBlock(id, _) => id,
Expand All @@ -219,7 +215,6 @@ impl<'ast> MapEntry<'ast> {
EntryVariant(_, n) => NodeVariant(n),
EntryExpr(_, n) => NodeExpr(n),
EntryStmt(_, n) => NodeStmt(n),
EntryArg(_, n) => NodeArg(n),
EntryLocal(_, n) => NodeLocal(n),
EntryPat(_, n) => NodePat(n),
EntryBlock(_, n) => NodeBlock(n),
Expand Down Expand Up @@ -348,6 +343,27 @@ impl<'ast> Map<'ast> {
self.find_entry(id).and_then(|x| x.parent_node()).unwrap_or(id)
}

/// Check if the node is an argument. An argument is a local variable whose
/// immediate parent is an item or a closure.
pub fn is_argument(&self, id: NodeId) -> bool {
match self.find(id) {
Some(NodeLocal(_)) => (),
_ => return false,
}
match self.find(self.get_parent_node(id)) {
Some(NodeItem(_)) |
Some(NodeTraitItem(_)) |
Some(NodeImplItem(_)) => true,
Some(NodeExpr(e)) => {
match e.node {
ExprClosure(..) => true,
_ => false,
}
}
_ => false,
}
}

/// If there is some error when walking the parents (e.g., a node does not
/// have a parent in the map or a node can't be found), then we return the
/// last good node id we found. Note that reaching the crate root (id == 0),
Expand Down Expand Up @@ -628,7 +644,7 @@ impl<'ast> Map<'ast> {
Some(NodeVariant(variant)) => variant.span,
Some(NodeExpr(expr)) => expr.span,
Some(NodeStmt(stmt)) => stmt.span,
Some(NodeArg(pat)) | Some(NodeLocal(pat)) => pat.span,
Some(NodeLocal(pat)) => pat.span,
Some(NodePat(pat)) => pat.span,
Some(NodeBlock(block)) => block.span,
Some(NodeStructCtor(_)) => self.expect_item(self.get_parent(id)).span,
Expand Down Expand Up @@ -886,7 +902,6 @@ impl<'a> NodePrinter for pprust::State<'a> {
// ast_map to reconstruct their full structure for pretty
// printing.
NodeLocal(_) => panic!("cannot print isolated Local"),
NodeArg(_) => panic!("cannot print isolated Arg"),
NodeStructCtor(_) => panic!("cannot print isolated StructCtor"),
}
}
Expand Down Expand Up @@ -965,9 +980,6 @@ fn node_id_to_string(map: &Map, id: NodeId, include_id: bool) -> String {
Some(NodeStmt(ref stmt)) => {
format!("stmt {}{}", pprust::stmt_to_string(&**stmt), id_str)
}
Some(NodeArg(ref pat)) => {
format!("arg {}{}", pprust::pat_to_string(&**pat), id_str)
}
Some(NodeLocal(ref pat)) => {
format!("local {}{}", pprust::pat_to_string(&**pat), id_str)
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ enum PassArgs {
impl<'d,'t,'a,'tcx> ExprUseVisitor<'d,'t,'a,'tcx> {
pub fn new(delegate: &'d mut (Delegate<'tcx>),
typer: &'t infer::InferCtxt<'a, 'tcx>)
-> ExprUseVisitor<'d,'t,'a,'tcx> where 'tcx:'a
-> ExprUseVisitor<'d,'t,'a,'tcx> where 'tcx:'a+'d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @nikomatsakis it's evolving?!

{
let mc: mc::MemCategorizationContext<'t, 'a, 'tcx> =
mc::MemCategorizationContext::new(typer);
Expand Down
11 changes: 5 additions & 6 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ impl MutabilityCategory {

fn from_local(tcx: &ty::ctxt, id: ast::NodeId) -> MutabilityCategory {
let ret = match tcx.map.get(id) {
ast_map::NodeLocal(p) | ast_map::NodeArg(p) => match p.node {
ast_map::NodeLocal(p) => match p.node {
hir::PatIdent(bind_mode, _, _) => {
if bind_mode == hir::BindByValue(hir::MutMutable) {
McDeclared
Expand Down Expand Up @@ -1463,11 +1463,10 @@ impl<'tcx> cmt_<'tcx> {
"non-lvalue".to_string()
}
cat_local(vid) => {
match tcx.map.find(vid) {
Some(ast_map::NodeArg(_)) => {
"argument".to_string()
}
_ => "local variable".to_string()
if tcx.map.is_argument(vid) {
"argument".to_string()
} else {
"local variable".to_string()
}
}
cat_deref(_, _, pk) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/trans/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1976,7 +1976,7 @@ pub fn create_captured_var_metadata<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
None => {
cx.sess().span_bug(span, "debuginfo::create_captured_var_metadata: node not found");
}
Some(hir_map::NodeLocal(pat)) | Some(hir_map::NodeArg(pat)) => {
Some(hir_map::NodeLocal(pat)) => {
match pat.node {
hir::PatIdent(_, ref path1, _) => {
path1.node.name
Expand Down
1 change: 0 additions & 1 deletion src/librustc_trans/trans/monomorphize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
hir_map::NodeTyParam(..) |
hir_map::NodeExpr(..) |
hir_map::NodeStmt(..) |
hir_map::NodeArg(..) |
hir_map::NodeBlock(..) |
hir_map::NodePat(..) |
hir_map::NodeLocal(..) => {
Expand Down
43 changes: 43 additions & 0 deletions src/test/compile-fail/borrowck-argument.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2015 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.

#[derive(Copy, Clone)]
struct S;

impl S {
fn mutate(&mut self) {
}
}

fn func(arg: S) {
arg.mutate(); //~ ERROR: cannot borrow immutable argument
}

impl S {
fn method(&self, arg: S) {
arg.mutate(); //~ ERROR: cannot borrow immutable argument
}
}

trait T {
fn default(&self, arg: S) {
arg.mutate(); //~ ERROR: cannot borrow immutable argument
}
}

impl T for S {}

fn main() {
let s = S;
func(s);
s.method(s);
s.default(s);
(|arg: S| { arg.mutate() })(s); //~ ERROR: cannot borrow immutable argument
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/borrowck-closures-unique.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn d(x: &mut isize) {
}

fn e(x: &mut isize) {
let c1 = || x = panic!(); //~ ERROR closure cannot assign to immutable local variable
let c1 = || x = panic!(); //~ ERROR closure cannot assign to immutable argument
}

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/borrowck-unboxed-closures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn a<F:Fn(isize, isize) -> isize>(mut f: F) {
}

fn b<F:FnMut(isize, isize) -> isize>(f: F) {
f(1, 2); //~ ERROR cannot borrow immutable local variable
f(1, 2); //~ ERROR cannot borrow immutable argument
}

fn c<F:FnOnce(isize, isize) -> isize>(f: F) {
Expand Down