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

Remove track_errors from check_match, typeck_item_bodies and register_plugins #59199

Merged
merged 5 commits into from
Mar 24, 2019
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
5 changes: 2 additions & 3 deletions src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ rustc_query_append! { [define_queries!][ <'tcx>

TypeChecking {
[] fn typeck_item_bodies:
typeck_item_bodies_dep_node(CrateNum) -> Result<(), ErrorReported>,
typeck_item_bodies_dep_node(CrateNum) -> (),

[] fn typeck_tables_of: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx>,
},
Expand Down Expand Up @@ -325,8 +325,7 @@ rustc_query_append! { [define_queries!][ <'tcx>
},

TypeChecking {
[] fn check_match: CheckMatch(DefId)
-> Result<(), ErrorReported>,
[] fn check_match: CheckMatch(DefId) -> (),

/// Performs part of the privacy check and computes "access levels".
[] fn privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Lrc<AccessLevels>,
Expand Down
26 changes: 12 additions & 14 deletions src/librustc_interface/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,22 +323,20 @@ pub fn register_plugins<'a>(
..
} = registry;

sess.track_errors(|| {
let mut ls = sess.lint_store.borrow_mut();
for pass in early_lint_passes {
ls.register_early_pass(Some(sess), true, false, pass);
}
for pass in late_lint_passes {
ls.register_late_pass(Some(sess), true, pass);
}
let mut ls = sess.lint_store.borrow_mut();
for pass in early_lint_passes {
ls.register_early_pass(Some(sess), true, false, pass);
}
for pass in late_lint_passes {
ls.register_late_pass(Some(sess), true, pass);
}

for (name, (to, deprecated_name)) in lint_groups {
ls.register_group(Some(sess), true, name, deprecated_name, to);
}
for (name, (to, deprecated_name)) in lint_groups {
ls.register_group(Some(sess), true, name, deprecated_name, to);
}

*sess.plugin_llvm_passes.borrow_mut() = llvm_passes;
*sess.plugin_attributes.borrow_mut() = attributes.clone();
})?;
*sess.plugin_llvm_passes.borrow_mut() = llvm_passes;
*sess.plugin_attributes.borrow_mut() = attributes.clone();

Ok((krate, PluginInfo {
syntax_exts,
Expand Down
14 changes: 10 additions & 4 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
self.simplify_candidate(&mut candidate);

if !candidate.match_pairs.is_empty() {
span_bug!(
// ICE if no other errors have been emitted. This used to be a hard error that wouldn't
// be reached because `hair::pattern::check_match::check_match` wouldn't have let the
// compiler continue. In our tests this is only ever hit by
// `ui/consts/const-match-check.rs` with `--cfg eval1`, and that file already generates
// a different error before hand.
estebank marked this conversation as resolved.
Show resolved Hide resolved
self.hir.tcx().sess.delay_span_bug(
candidate.match_pairs[0].pattern.span,
"match pairs {:?} remaining after simplifying \
irrefutable pattern",
candidate.match_pairs
&format!(
"match pairs {:?} remaining after simplifying irrefutable pattern",
candidate.match_pairs,
),
);
}

Expand Down
6 changes: 2 additions & 4 deletions src/librustc_mir/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use rustc::ty::layout::{self, LayoutOf, VariantIdx};
use rustc::ty::subst::Subst;
use rustc::traits::Reveal;
use rustc_data_structures::fx::FxHashMap;
use rustc::util::common::ErrorReported;

use syntax::ast::Mutability;
use syntax::source_map::{Span, DUMMY_SP};
Expand Down Expand Up @@ -619,9 +618,8 @@ pub fn const_eval_raw_provider<'a, 'tcx>(
let tables = tcx.typeck_tables_of(def_id);

// Do match-check before building MIR
if let Err(ErrorReported) = tcx.check_match(def_id) {
return Err(ErrorHandled::Reported)
}
// FIXME(#59378) check_match may have errored but we're not checking for that anymore
tcx.check_match(def_id);
Copy link
Member

@eddyb eddyb Mar 18, 2019

Choose a reason for hiding this comment

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

Calling this feels pointless, also similar for mir_const_qualif below.
cc @oli-obk Can we just remove these and rely only on tainted_by_errors instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I seem to remember that building MIR before match checking results in ICEs. Not sure this belongs here though.

Copy link
Member

Choose a reason for hiding this comment

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

That shouldn't make sense nowadays, though, there should be no mutable state involved?

Copy link
Contributor

Choose a reason for hiding this comment

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

As in check_match reports errors for match expressions while MIR building ICEs on those same expressions.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, sure, but that means MIR building should be handling this gracefully.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you ok with this being addressed in a separate PR? If so, I'll open an issue

Copy link
Member

Choose a reason for hiding this comment

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

It should be handled here, as the behavior of "exit if check_match errored" is gone! You could try to turn an error into an ICE if you want.

Copy link
Member

Choose a reason for hiding this comment

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

But I guess I'd be fine with a // FIXME(#...) check_match may have errored but we're not checking for that anymore here.


if let hir::BodyOwnerKind::Const = tcx.hir().body_owner_kind_by_hir_id(id) {
tcx.mir_const_qualif(def_id);
Expand Down
24 changes: 9 additions & 15 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use rustc::ty::{self, Ty, TyCtxt, TyKind};
use rustc::ty::subst::{InternalSubsts, SubstsRef};
use rustc::lint;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc::util::common::ErrorReported;

use rustc::hir::def::*;
use rustc::hir::def_id::DefId;
Expand All @@ -27,25 +26,20 @@ use std::slice;
use syntax::ptr::P;
use syntax_pos::{Span, DUMMY_SP, MultiSpan};

pub(crate) fn check_match<'a, 'tcx>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId,
) -> Result<(), ErrorReported> {
pub(crate) fn check_match<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
let body_id = if let Some(id) = tcx.hir().as_local_hir_id(def_id) {
tcx.hir().body_owned_by(id)
} else {
return Ok(());
return;
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 Is this intentional? I remember some of the queries I wrote would panic if they got a non-local DefId.

};

tcx.sess.track_errors(|| {
MatchVisitor {
tcx,
tables: tcx.body_tables(body_id),
region_scope_tree: &tcx.region_scope_tree(def_id),
param_env: tcx.param_env(def_id),
identity_substs: InternalSubsts::identity_for_item(tcx, def_id),
}.visit_body(tcx.hir().body(body_id));
})
MatchVisitor {
tcx,
tables: tcx.body_tables(body_id),
region_scope_tree: &tcx.region_scope_tree(def_id),
param_env: tcx.param_env(def_id),
identity_substs: InternalSubsts::identity_for_item(tcx, def_id),
}.visit_body(tcx.hir().body(body_id));
}

fn create_e0004<'a>(sess: &'a Session, sp: Span, error_message: String) -> DiagnosticBuilder<'a> {
Expand Down
12 changes: 4 additions & 8 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,15 +702,11 @@ fn check_mod_item_types<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId)
tcx.hir().visit_item_likes_in_module(module_def_id, &mut CheckItemTypesVisitor { tcx });
}

fn typeck_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum)
-> Result<(), ErrorReported>
{
fn typeck_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum) {
debug_assert!(crate_num == LOCAL_CRATE);
Ok(tcx.sess.track_errors(|| {
tcx.par_body_owners(|body_owner_def_id| {
tcx.ensure().typeck_tables_of(body_owner_def_id);
});
})?)
tcx.par_body_owners(|body_owner_def_id| {
tcx.ensure().typeck_tables_of(body_owner_def_id);
});
}

fn check_item_well_formed<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
})
})?;

time(tcx.sess, "item-bodies checking", || tcx.typeck_item_bodies(LOCAL_CRATE))?;
time(tcx.sess, "item-bodies checking", || tcx.typeck_item_bodies(LOCAL_CRATE));

check_unused::check_crate(tcx);
check_for_entry_fn(tcx);
Expand Down
6 changes: 5 additions & 1 deletion src/test/ui/issues/issue-26217.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
fn foo<T>() where for<'a> T: 'a {}

fn main<'a>() {
fn bar<'a>() {
foo::<&'a i32>();
//~^ ERROR the type `&'a i32` does not fulfill the required lifetime
}

fn main() {
bar();
}