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

Make generic parameters always use modern hygiene #63083

Merged
merged 3 commits into from
Jul 30, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2568,7 +2568,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
let lifetimes: Vec<_> = params
.iter()
.filter_map(|param| match param.kind {
GenericParamKind::Lifetime { .. } => Some((param, param.name)),
GenericParamKind::Lifetime { .. } => Some((param, param.name.modern())),
_ => None,
})
.collect();
Expand Down
26 changes: 18 additions & 8 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,8 +872,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> {
debug!("(resolving function) entering function");
let rib_kind = match function_kind {
FnKind::ItemFn(..) => FnItemRibKind,
FnKind::Method(..) => AssocItemRibKind,
FnKind::Closure(_) => NormalRibKind,
FnKind::Method(..) | FnKind::Closure(_) => NormalRibKind,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this changed, and why did it work previously?
I don't quite remember all the differences between many kinds of ribs that act as "barriers" for names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the reason that this has little effect is that there is already an AssocItemRib around all associated items that holds any generic parameters. Since generic parameters can be seen through associated items, this rib isn't stopping any names from being visible.
The reason for the change is so that function parameters don't end up in an associated item rib, and get resolved with the wrong hygiene.

};

// Create a value rib for the function.
Expand Down Expand Up @@ -2310,21 +2309,32 @@ impl<'a> Resolver<'a> {
if ident.name == kw::Invalid {
return Some(LexicalScopeBinding::Res(Res::Err));
}
ident.span = if ident.name == kw::SelfUpper {
let (general_span, modern_span) = if ident.name == kw::SelfUpper {
// FIXME(jseyfried) improve `Self` hygiene
ident.span.with_ctxt(SyntaxContext::empty())
let empty_span = ident.span.with_ctxt(SyntaxContext::empty());
(empty_span, empty_span)
} else if ns == TypeNS {
ident.span.modern()
let modern_span = ident.span.modern();
(modern_span, modern_span)
} else {
ident.span.modern_and_legacy()
(ident.span.modern_and_legacy(), ident.span.modern())
};
ident.span = general_span;
let modern_ident = Ident { span: modern_span, ..ident };

// Walk backwards up the ribs in scope.
let record_used = record_used_id.is_some();
let mut module = self.graph_root;
for i in (0 .. self.ribs[ns].len()).rev() {
debug!("walk rib\n{:?}", self.ribs[ns][i].bindings);
if let Some(res) = self.ribs[ns][i].bindings.get(&ident).cloned() {
// Use the rib kind to determine whether we are resolving parameters
// (modern hygiene) or local variables (legacy hygiene).
let rib_ident = if let AssocItemRibKind | ItemRibKind = self.ribs[ns][i].kind {
modern_ident
} else {
ident
};
if let Some(res) = self.ribs[ns][i].bindings.get(&rib_ident).cloned() {
// The ident resolves to a type parameter or local variable.
return Some(LexicalScopeBinding::Res(
self.validate_res_from_ribs(ns, i, res, record_used, path_span),
Expand Down Expand Up @@ -2360,7 +2370,7 @@ impl<'a> Resolver<'a> {
}
}

ident.span = ident.span.modern();
ident = modern_ident;
let mut poisoned = None;
loop {
let opt_module = if let Some(node_id) = record_used_id {
Expand Down
19 changes: 19 additions & 0 deletions src/test/ui/hygiene/duplicate_lifetimes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Ensure that lifetime parameter names are modernized before we check for
// duplicates.

#![feature(decl_macro, rustc_attrs)]

#[rustc_macro_transparency = "semitransparent"]
macro m($a:lifetime) {
fn g<$a, 'a>() {} //~ ERROR lifetime name `'a` declared twice
}

#[rustc_macro_transparency = "transparent"]
macro n($a:lifetime) {
fn h<$a, 'a>() {} //~ ERROR lifetime name `'a` declared twice
}

m!('a);
n!('a);

fn main() {}
27 changes: 27 additions & 0 deletions src/test/ui/hygiene/duplicate_lifetimes.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error[E0263]: lifetime name `'a` declared twice in the same scope
--> $DIR/duplicate_lifetimes.rs:8:14
|
LL | fn g<$a, 'a>() {}
| ^^ declared twice
...
LL | m!('a);
| -------
| | |
| | previous declaration here
| in this macro invocation

error[E0263]: lifetime name `'a` declared twice in the same scope
--> $DIR/duplicate_lifetimes.rs:13:14
|
LL | fn h<$a, 'a>() {}
| ^^ declared twice
...
LL | n!('a);
| -------
| | |
| | previous declaration here
| in this macro invocation

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0263`.
104 changes: 104 additions & 0 deletions src/test/ui/hygiene/generic_params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
// Ensure that generic parameters always have modern hygiene.

// check-pass
// ignore-pretty pretty-printing is unhygienic

#![feature(decl_macro, rustc_attrs, const_generics)]

mod type_params {
macro m($T:ident) {
fn f<$T: Clone, T: PartialEq>(t1: $T, t2: T) -> ($T, bool) {
(t1.clone(), t2 == t2)
}
}

#[rustc_macro_transparency = "semitransparent"]
macro n($T:ident) {
fn g<$T: Clone>(t1: $T, t2: T) -> (T, $T) {
(t1.clone(), t2.clone())
}
fn h<T: Clone>(t1: $T, t2: T) -> (T, $T) {
(t1.clone(), t2.clone())
}
}

#[rustc_macro_transparency = "transparent"]
macro p($T:ident) {
fn j<$T: Clone>(t1: $T, t2: T) -> (T, $T) {
(t1.clone(), t2.clone())
}
fn k<T: Clone>(t1: $T, t2: T) -> (T, $T) {
(t1.clone(), t2.clone())
}
}

m!(T);
n!(T);
p!(T);
}

mod lifetime_params {
macro m($a:lifetime) {
fn f<'b, 'c, $a: 'b, 'a: 'c>(t1: &$a(), t2: &'a ()) -> (&'b (), &'c ()) {
(t1, t2)
}
}

#[rustc_macro_transparency = "semitransparent"]
macro n($a:lifetime) {
fn g<$a>(t1: &$a(), t2: &'a ()) -> (&'a (), &$a ()) {
(t1, t2)
}
fn h<'a>(t1: &$a(), t2: &'a ()) -> (&'a (), &$a ()) {
(t1, t2)
}
}

#[rustc_macro_transparency = "transparent"]
macro p($a:lifetime) {
fn j<$a>(t1: &$a(), t2: &'a ()) -> (&'a (), &$a ()) {
(t1, t2)
}
fn k<'a>(t1: &$a(), t2: &'a ()) -> (&'a (), &$a ()) {
(t1, t2)
}
}

m!('a);
n!('a);
p!('a);
}

mod const_params {
macro m($C:ident) {
fn f<const $C: usize, const C: usize>(t1: [(); $C], t2: [(); C]) -> ([(); $C], [(); C]) {
(t1, t2)
}
}

#[rustc_macro_transparency = "semitransparent"]
macro n($C:ident) {
fn g<const $C: usize>(t1: [(); $C], t2: [(); C]) -> ([(); C], [(); $C]) {
(t1, t2)
}
fn h<const C: usize>(t1: [(); $C], t2: [(); C]) -> ([(); C], [(); $C]) {
(t1, t2)
}
}

#[rustc_macro_transparency = "transparent"]
macro p($C:ident) {
fn j<const $C: usize>(t1: [(); $C], t2: [(); C]) -> ([(); C], [(); $C]) {
(t1, t2)
}
fn k<const C: usize>(t1: [(); $C], t2: [(); C]) -> ([(); C], [(); $C]) {
(t1, t2)
}
}

m!(C);
n!(C);
p!(C);
}

fn main() {}
6 changes: 6 additions & 0 deletions src/test/ui/hygiene/generic_params.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
warning: the feature `const_generics` is incomplete and may cause the compiler to crash
--> $DIR/generic_params.rs:6:37
|
LL | #![feature(decl_macro, rustc_attrs, const_generics)]
| ^^^^^^^^^^^^^^

14 changes: 0 additions & 14 deletions src/test/ui/hygiene/ty_params.rs

This file was deleted.