Skip to content

Commit

Permalink
librustc: Forbid duplicate name bindings in the same parameter or type
Browse files Browse the repository at this point in the history
parameter list.

This breaks code like:

    fn f(a: int, a: int) { ... }
    fn g<T,T>(a: T) { ... }

Change this code to not use the same name for a parameter. For example:

    fn f(a: int, b: int) { ... }
    fn g<T,U>(a: T) { ... }

Code like this is *not* affected, since `_` is not an identifier:

    fn f(_: int, _: int) { ... } // OK

Closes #17568.

[breaking-change]
  • Loading branch information
pcwalton committed Oct 9, 2014
1 parent 63fe80e commit 1498814
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 6 deletions.
34 changes: 28 additions & 6 deletions src/librustc/middle/resolve.rs
Expand Up @@ -4232,15 +4232,25 @@ impl<'a> Resolver<'a> {
type_parameters: TypeParameters,
f: |&mut Resolver|) {
match type_parameters {
HasTypeParameters(generics, space, node_id,
rib_kind) => {

HasTypeParameters(generics, space, node_id, rib_kind) => {
let mut function_type_rib = Rib::new(rib_kind);

let mut seen_bindings = HashSet::new();
for (index, type_parameter) in generics.ty_params.iter().enumerate() {
let ident = type_parameter.ident;
debug!("with_type_parameter_rib: {} {}", node_id,
type_parameter.id);

if seen_bindings.contains(&ident) {
self.resolve_error(type_parameter.span,
format!("the name `{}` is already \
used for a type \
parameter in this type \
parameter list",
token::get_ident(
ident)).as_slice())
}
seen_bindings.insert(ident);

let def_like = DlDef(DefTyParam(space,
local_def(type_parameter.id),
index));
Expand Down Expand Up @@ -4313,8 +4323,8 @@ impl<'a> Resolver<'a> {
// Nothing to do.
}
Some(declaration) => {
let mut bindings_list = HashMap::new();
for argument in declaration.inputs.iter() {
let mut bindings_list = HashMap::new();
this.resolve_pattern(&*argument.pat,
ArgumentIrrefutableMode,
&mut bindings_list);
Expand Down Expand Up @@ -5056,12 +5066,24 @@ impl<'a> Resolver<'a> {
// must not add it if it's in the bindings list
// because that breaks the assumptions later
// passes make about or-patterns.)

if !bindings_list.contains_key(&renamed) {
let this = &mut *self;
let last_rib = this.value_ribs.last_mut().unwrap();
last_rib.bindings.insert(renamed, DlDef(def));
bindings_list.insert(renamed, pat_id);
} else if mode == ArgumentIrrefutableMode &&
bindings_list.contains_key(&renamed) {
// Forbid duplicate bindings in the same
// parameter list.
self.resolve_error(pattern.span,
format!("identifier `{}` \
is bound more \
than once in \
this parameter \
list",
token::get_ident(
ident))
.as_slice())
} else if bindings_list.find(&renamed) ==
Some(&pat_id) {
// Then this is a duplicate variable in the
Expand Down
16 changes: 16 additions & 0 deletions src/test/compile-fail/duplicate-parameter.rs
@@ -0,0 +1,16 @@
// 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 <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.

fn f(a: int, a: int) {}
//~^ ERROR identifier `a` is bound more than once in this parameter list

fn main() {
}

39 changes: 39 additions & 0 deletions src/test/compile-fail/duplicate-type-parameter.rs
@@ -0,0 +1,39 @@
// 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 <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.

type Foo<T,T> = Option<T>;
//~^ ERROR the name `T` is already used

struct Bar<T,T>(T);
//~^ ERROR the name `T` is already used

struct Baz<T,T> {
//~^ ERROR the name `T` is already used
x: T,
}

enum Boo<T,T> {
//~^ ERROR the name `T` is already used
A(T),
B,
}

fn quux<T,T>(x: T) {}
//~^ ERROR the name `T` is already used

trait Qux<T,T> {}
//~^ ERROR the name `T` is already used

impl<T,T> Qux<T,T> for Option<T> {}
//~^ ERROR the name `T` is already used

fn main() {
}

8 comments on commit 1498814

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

saw approval from alexcrichton
at pcwalton@1498814

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

merging pcwalton/rust/duplicate-bindings-in-parameter-list = 1498814 into auto

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

pcwalton/rust/duplicate-bindings-in-parameter-list = 1498814 merged ok, testing candidate = df0f9fce

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

No active merge of candidate 1498814 found, likely manual push to master

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

merging pcwalton/rust/duplicate-bindings-in-parameter-list = 1498814 into auto

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

pcwalton/rust/duplicate-bindings-in-parameter-list = 1498814 merged ok, testing candidate = eb04229

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

@bors
Copy link
Contributor

@bors bors commented on 1498814 Oct 9, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = eb04229

Please sign in to comment.