Skip to content

Commit

Permalink
Rollup merge of #68399 - Centril:check-match-unify, r=oli-obk
Browse files Browse the repository at this point in the history
check_match: misc unifications and ICE fixes

These are some unifications made as a by-product of working on `hir::ExprKind::Let`.

Fixes #68396.
Fixes #68394.
Fixes #68393.

r? @oli-obk @matthewjasper
  • Loading branch information
Centril committed Jan 21, 2020
2 parents 3fa8cc3 + 58eb03d commit 5850482
Show file tree
Hide file tree
Showing 13 changed files with 198 additions and 95 deletions.
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`

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

0 comments on commit 5850482

Please sign in to comment.