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

check_match: misc unifications and ICE fixes #68399

Merged
merged 3 commits into from
Jan 21, 2020
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3763,6 +3763,7 @@ dependencies = [
"rustc_hir",
"rustc_index",
"rustc_macros",
"rustc_session",
"rustc_span",
"rustc_target",
"serialize",
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir_build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ rustc_errors = { path = "../librustc_errors" }
rustc_hir = { path = "../librustc_hir" }
rustc_macros = { path = "../librustc_macros" }
rustc_serialize = { path = "../libserialize", package = "serialize" }
rustc_session = { path = "../librustc_session" }
rustc_span = { path = "../librustc_span" }
rustc_target = { path = "../librustc_target" }
syntax = { path = "../libsyntax" }
Expand Down
9 changes: 3 additions & 6 deletions src/librustc_mir_build/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -582,15 +582,12 @@ crate struct MatchCheckCtxt<'a, 'tcx> {
}

impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> {
crate fn create_and_enter<F, R>(
crate fn create_and_enter<R>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
module: DefId,
f: F,
) -> R
where
F: for<'b> FnOnce(MatchCheckCtxt<'b, 'tcx>) -> R,
{
f: impl for<'b> FnOnce(MatchCheckCtxt<'b, 'tcx>) -> R,
) -> R {
let pattern_arena = TypedArena::default();

f(MatchCheckCtxt { tcx, param_env, module, pattern_arena: &pattern_arena })
Expand Down
153 changes: 66 additions & 87 deletions src/librustc_mir_build/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,17 @@ use super::_match::{expand_pattern, is_useful, MatchCheckCtxt, Matrix, PatStack}
use super::{PatCtxt, PatKind, PatternError};

use rustc::hir::map::Map;
use rustc::lint;
use rustc::session::parse::feature_err;
use rustc::session::Session;
use rustc::ty::{self, Ty, TyCtxt};
use rustc_errors::{error_code, struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def::*;
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{HirId, Pat};
use rustc_session::lint::builtin::BINDINGS_WITH_VARIANT_NAME;
use rustc_session::lint::builtin::{IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS};
use rustc_session::parse::feature_err;
use rustc_session::Session;
use rustc_span::symbol::sym;
use rustc_span::{MultiSpan, Span};
use syntax::ast::Mutability;
Expand Down Expand Up @@ -67,18 +68,13 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> {
hir::LocalSource::AwaitDesugar => ("`await` future binding", None),
};
self.check_irrefutable(&loc.pat, msg, sp);

// Check legality of move bindings and `@` patterns.
self.check_patterns(false, &loc.pat);
}

fn visit_body(&mut self, body: &'tcx hir::Body<'tcx>) {
intravisit::walk_body(self, body);

for param in body.params {
self.check_irrefutable(&param.pat, "function argument", None);
self.check_patterns(false, &param.pat);
}
fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
intravisit::walk_param(self, param);
self.check_irrefutable(&param.pat, "function argument", None);
self.check_patterns(false, &param.pat);
}
}

Expand Down Expand Up @@ -123,6 +119,25 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
if !self.tcx.features().bindings_after_at {
check_legality_of_bindings_in_at_patterns(self, pat);
}
check_for_bindings_named_same_as_variants(self, pat);
}

fn lower_pattern<'p>(
&self,
cx: &mut MatchCheckCtxt<'p, 'tcx>,
pat: &'tcx hir::Pat<'tcx>,
have_errors: &mut bool,
) -> (&'p super::Pat<'tcx>, Ty<'tcx>) {
let mut patcx = PatCtxt::new(self.tcx, self.param_env, self.tables);
patcx.include_lint_checks();
let pattern = patcx.lower_pattern(pat);
let pattern_ty = pattern.ty;
let pattern: &_ = cx.pattern_arena.alloc(expand_pattern(cx, pattern));
if !patcx.errors.is_empty() {
*have_errors = true;
patcx.report_inlining_errors(pat.span);
}
(pattern, pattern_ty)
}

fn check_match(
Expand All @@ -132,11 +147,8 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
source: hir::MatchSource,
) {
for arm in arms {
// First, check legality of move bindings.
// Check the arm for some things unrelated to exhaustiveness.
self.check_patterns(arm.guard.is_some(), &arm.pat);

// Second, perform some lints.
check_for_bindings_named_same_as_variants(self, &arm.pat);
}

let module = self.tcx.hir().get_module_parent(scrut.hir_id);
Expand All @@ -145,16 +157,8 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {

let inlined_arms: Vec<_> = arms
.iter()
.map(|arm| {
let mut patcx = PatCtxt::new(self.tcx, self.param_env, self.tables);
patcx.include_lint_checks();
let pattern = patcx.lower_pattern(&arm.pat);
let pattern: &_ = cx.pattern_arena.alloc(expand_pattern(cx, pattern));
if !patcx.errors.is_empty() {
patcx.report_inlining_errors(arm.pat.span);
have_errors = true;
}
(pattern, &*arm.pat, arm.guard.is_some())
.map(|hir::Arm { pat, guard, .. }| {
(self.lower_pattern(cx, pat, &mut have_errors).0, pat.hir_id, guard.is_some())
})
.collect();

Expand All @@ -178,11 +182,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> {
fn check_irrefutable(&self, pat: &'tcx Pat<'tcx>, origin: &str, sp: Option<Span>) {
let module = self.tcx.hir().get_module_parent(pat.hir_id);
MatchCheckCtxt::create_and_enter(self.tcx, self.param_env, module, |ref mut cx| {
let mut patcx = PatCtxt::new(self.tcx, self.param_env, self.tables);
patcx.include_lint_checks();
let pattern = patcx.lower_pattern(pat);
let pattern_ty = pattern.ty;
let pattern = cx.pattern_arena.alloc(expand_pattern(cx, pattern));
let (pattern, pattern_ty) = self.lower_pattern(cx, pat, &mut false);
let pats: Matrix<'_, '_> = vec![PatStack::from_pattern(pattern)].into_iter().collect();

let witnesses = match check_not_useful(cx, pattern_ty, &pats, pat.hir_id) {
Expand Down Expand Up @@ -285,7 +285,7 @@ fn check_for_bindings_named_same_as_variants(cx: &MatchVisitor<'_, '_>, pat: &Pa
let ty_path = cx.tcx.def_path_str(edef.did);
cx.tcx
.struct_span_lint_hir(
lint::builtin::BINDINGS_WITH_VARIANT_NAME,
BINDINGS_WITH_VARIANT_NAME,
p.hir_id,
p.span,
&format!(
Expand All @@ -310,79 +310,63 @@ fn check_for_bindings_named_same_as_variants(cx: &MatchVisitor<'_, '_>, pat: &Pa
}

/// Checks for common cases of "catchall" patterns that may not be intended as such.
fn pat_is_catchall(pat: &Pat<'_>) -> bool {
match pat.kind {
hir::PatKind::Binding(.., None) => true,
hir::PatKind::Binding(.., Some(ref s)) => pat_is_catchall(s),
hir::PatKind::Ref(ref s, _) => pat_is_catchall(s),
hir::PatKind::Tuple(ref v, _) => v.iter().all(|p| pat_is_catchall(&p)),
fn pat_is_catchall(pat: &super::Pat<'_>) -> bool {
use super::PatKind::*;
match &*pat.kind {
Binding { subpattern: None, .. } => true,
Binding { subpattern: Some(s), .. } | Deref { subpattern: s } => pat_is_catchall(s),
Leaf { subpatterns: s } => s.iter().all(|p| pat_is_catchall(&p.pattern)),
_ => false,
}
}

fn unreachable_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, catchall: Option<Span>) {
let mut err = tcx.struct_span_lint_hir(UNREACHABLE_PATTERNS, id, span, "unreachable pattern");
if let Some(catchall) = catchall {
// We had a catchall pattern, hint at that.
err.span_label(span, "unreachable pattern");
err.span_label(catchall, "matches any value");
}
err.emit();
}

fn irrefutable_let_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, source: hir::MatchSource) {
let msg = match source {
hir::MatchSource::IfLetDesugar { .. } => "irrefutable if-let pattern",
hir::MatchSource::WhileLetDesugar => "irrefutable while-let pattern",
_ => bug!(),
};
tcx.lint_hir(IRREFUTABLE_LET_PATTERNS, id, span, msg);
}

/// Check for unreachable patterns.
fn check_arms<'p, 'tcx>(
cx: &mut MatchCheckCtxt<'p, 'tcx>,
arms: &[(&'p super::Pat<'tcx>, &hir::Pat<'_>, bool)],
arms: &[(&'p super::Pat<'tcx>, HirId, bool)],
source: hir::MatchSource,
) -> Matrix<'p, 'tcx> {
let mut seen = Matrix::empty();
let mut catchall = None;
for (arm_index, (pat, hir_pat, has_guard)) in arms.iter().enumerate() {
for (arm_index, (pat, id, has_guard)) in arms.iter().copied().enumerate() {
let v = PatStack::from_pattern(pat);

match is_useful(cx, &seen, &v, LeaveOutWitness, hir_pat.hir_id, true) {
match is_useful(cx, &seen, &v, LeaveOutWitness, id, true) {
NotUseful => {
match source {
hir::MatchSource::IfDesugar { .. } | hir::MatchSource::WhileDesugar => bug!(),

hir::MatchSource::IfLetDesugar { .. } | hir::MatchSource::WhileLetDesugar => {
// check which arm we're on.
// Check which arm we're on.
match arm_index {
// The arm with the user-specified pattern.
0 => {
cx.tcx.lint_hir(
lint::builtin::UNREACHABLE_PATTERNS,
hir_pat.hir_id,
pat.span,
"unreachable pattern",
);
}
0 => unreachable_pattern(cx.tcx, pat.span, id, None),
// The arm with the wildcard pattern.
1 => {
let msg = match source {
hir::MatchSource::IfLetDesugar { .. } => {
"irrefutable if-let pattern"
}
hir::MatchSource::WhileLetDesugar => {
"irrefutable while-let pattern"
}
_ => bug!(),
};
cx.tcx.lint_hir(
lint::builtin::IRREFUTABLE_LET_PATTERNS,
hir_pat.hir_id,
pat.span,
msg,
);
}
1 => irrefutable_let_pattern(cx.tcx, pat.span, id, source),
_ => bug!(),
}
}

hir::MatchSource::ForLoopDesugar | hir::MatchSource::Normal => {
let mut err = cx.tcx.struct_span_lint_hir(
lint::builtin::UNREACHABLE_PATTERNS,
hir_pat.hir_id,
pat.span,
"unreachable pattern",
);
// if we had a catchall pattern, hint at that
if let Some(catchall) = catchall {
err.span_label(pat.span, "unreachable pattern");
err.span_label(catchall, "matches any value");
}
err.emit();
unreachable_pattern(cx.tcx, pat.span, id, catchall);
}

// Unreachable patterns in try and await expressions occur when one of
Expand All @@ -392,19 +376,14 @@ fn check_arms<'p, 'tcx>(
}
Useful(unreachable_subpatterns) => {
for pat in unreachable_subpatterns {
cx.tcx.lint_hir(
lint::builtin::UNREACHABLE_PATTERNS,
hir_pat.hir_id,
pat.span,
"unreachable pattern",
);
unreachable_pattern(cx.tcx, pat.span, id, None);
}
}
UsefulWithWitness(_) => bug!(),
}
if !has_guard {
seen.push(v);
if catchall.is_none() && pat_is_catchall(hir_pat) {
if catchall.is_none() && pat_is_catchall(pat) {
catchall = Some(pat.span);
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/lint/lint-uppercase-variables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ fn main() {
//~^^^ WARN unused variable: `Foo`
}

let Foo = foo::Foo::Foo;
//~^ ERROR variable `Foo` should have a snake case name
//~^^ WARN `Foo` is named the same as one of the variants of the type `foo::Foo`
//~^^^ WARN unused variable: `Foo`

fn in_param(Foo: foo::Foo) {}
//~^ ERROR variable `Foo` should have a snake case name
//~^^ WARN `Foo` is named the same as one of the variants of the type `foo::Foo`
//~^^^ WARN unused variable: `Foo`
Comment on lines +34 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use //~| in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mostly copied existing code. Should we file an issue for follow-up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nah, its fine.


test(1);

let _ = Something { X: 0 };
Expand Down
38 changes: 37 additions & 1 deletion src/test/ui/lint/lint-uppercase-variables.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ LL | Foo => {}
|
= note: `#[warn(bindings_with_variant_name)]` on by default

warning[E0170]: pattern binding `Foo` is named the same as one of the variants of the type `foo::Foo`
--> $DIR/lint-uppercase-variables.rs:28:9
|
LL | let Foo = foo::Foo::Foo;
| ^^^ help: to match on the variant, qualify the path: `foo::Foo::Foo`

warning[E0170]: pattern binding `Foo` is named the same as one of the variants of the type `foo::Foo`
--> $DIR/lint-uppercase-variables.rs:33:17
|
LL | fn in_param(Foo: foo::Foo) {}
| ^^^ help: to match on the variant, qualify the path: `foo::Foo::Foo`

warning: unused variable: `Foo`
--> $DIR/lint-uppercase-variables.rs:22:9
|
Expand All @@ -19,6 +31,18 @@ LL | #![warn(unused)]
| ^^^^^^
= note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`

warning: unused variable: `Foo`
--> $DIR/lint-uppercase-variables.rs:28:9
|
LL | let Foo = foo::Foo::Foo;
| ^^^ help: consider prefixing with an underscore: `_Foo`

warning: unused variable: `Foo`
--> $DIR/lint-uppercase-variables.rs:33:17
|
LL | fn in_param(Foo: foo::Foo) {}
| ^^^ help: consider prefixing with an underscore: `_Foo`

error: structure field `X` should have a snake case name
--> $DIR/lint-uppercase-variables.rs:10:5
|
Expand Down Expand Up @@ -49,6 +73,18 @@ error: variable `Foo` should have a snake case name
LL | Foo => {}
| ^^^ help: convert the identifier to snake case (notice the capitalization): `foo`

error: aborting due to 4 previous errors
error: variable `Foo` should have a snake case name
--> $DIR/lint-uppercase-variables.rs:28:9
|
LL | let Foo = foo::Foo::Foo;
| ^^^ help: convert the identifier to snake case (notice the capitalization): `foo`

error: variable `Foo` should have a snake case name
--> $DIR/lint-uppercase-variables.rs:33:17
|
LL | fn in_param(Foo: foo::Foo) {}
| ^^^ help: convert the identifier to snake case (notice the capitalization): `foo`

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0170`.
26 changes: 26 additions & 0 deletions src/test/ui/pattern/issue-68393-let-pat-assoc-constant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
pub enum EFoo {
A,
}

pub trait Foo {
const X: EFoo;
}

struct Abc;

impl Foo for Abc {
const X: EFoo = EFoo::A;
}

struct Def;
impl Foo for Def {
const X: EFoo = EFoo::A;
}

pub fn test<A: Foo, B: Foo>(arg: EFoo, A::X: EFoo) {
//~^ ERROR associated consts cannot be referenced in patterns
let A::X = arg;
//~^ ERROR associated consts cannot be referenced in patterns
}

fn main() {}
Loading