Skip to content

address more FIXME whose associated issues were marked as closed #45097

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

Merged
merged 3 commits into from
Oct 17, 2017
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: 0 additions & 5 deletions src/libcore/tests/num/flt2dec/estimator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// FIXME https://github.com/kripken/emscripten/issues/4563
// NB we have to actually not compile this test to avoid
// an undefined symbol error
#![cfg(not(target_os = "emscripten"))]

use core::num::flt2dec::estimator::*;

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ fn build_local_id_to_index(body: Option<&hir::Body>,
-> FxHashMap<hir::ItemLocalId, Vec<CFGIndex>> {
let mut index = FxHashMap();

// FIXME (#6298): Would it be better to fold formals from decl
// FIXME(#15020) Would it be better to fold formals from decl
// into cfg itself? i.e. introduce a fn-based flow-graph in
// addition to the current block-based flow-graph, rather than
// have to put traversals like this here?
Expand Down
6 changes: 2 additions & 4 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,10 +477,8 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {

fn pat_ty(&self, pat: &hir::Pat) -> McResult<Ty<'tcx>> {
let base_ty = self.node_ty(pat.hir_id)?;
// FIXME (Issue #18207): This code detects whether we are
// looking at a `ref x`, and if so, figures out what the type
// *being borrowed* is. But ideally we would put in a more
// fundamental fix to this conflated use of the node id.
// This code detects whether we are looking at a `ref x`,
// and if so, figures out what the type *being borrowed* is.
let ret_ty = match pat.node {
PatKind::Binding(..) => {
let bm = *self.tables
Expand Down
4 changes: 1 addition & 3 deletions src/librustc/middle/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr:

hir::ExprAssignOp(..) | hir::ExprIndex(..) |
hir::ExprUnary(..) | hir::ExprCall(..) | hir::ExprMethodCall(..) => {
// FIXME(#6268) Nested method calls
// FIXME(https://github.com/rust-lang/rfcs/issues/811) Nested method calls
//
// The lifetimes for a call or method call look as follows:
//
Expand Down Expand Up @@ -1081,8 +1081,6 @@ fn resolve_local<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>,
// Here, the expression `[...]` has an extended lifetime due to rule
// A, but the inner rvalues `a()` and `b()` have an extended lifetime
// due to rule C.
//
// FIXME(#6308) -- Note that `[]` patterns work more smoothly post-DST.

if let Some(expr) = init {
record_rvalue_scope_if_borrow_expr(visitor, &expr, blk_scope);
Expand Down
2 changes: 0 additions & 2 deletions src/librustc/session/filesearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ pub enum FileMatch {
}

// A module for searching for libraries
// FIXME (#2658): I'm not happy how this module turned out. Should
// probably just be folded into cstore.

pub struct FileSearch<'a> {
pub sysroot: &'a Path,
Expand Down
1 change: 0 additions & 1 deletion src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ fn uncovered_tys<'tcx>(tcx: TyCtxt, ty: Ty<'tcx>, infer_is_local: InferIsLocal)

fn is_type_parameter(ty: Ty) -> bool {
match ty.sty {
// FIXME(#20590) straighten story about projection types
ty::TyProjection(..) | ty::TyParam(..) => true,
_ => false,
}
Expand Down
14 changes: 2 additions & 12 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1309,13 +1309,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
};

if obligation.predicate.skip_binder().self_ty().is_ty_var() {
// FIXME(#20297): Self is a type variable (e.g. `_: AsRef<str>`).
// Self is a type variable (e.g. `_: AsRef<str>`).
//
// This is somewhat problematic, as the current scheme can't really
// handle it turning to be a projection. This does end up as truly
// ambiguous in most cases anyway.
//
// Until this is fixed, take the fast path out - this also improves
// Take the fast path out - this also improves
// performance by preventing assemble_candidates_from_impls from
// matching every impl for this trait.
return Ok(SelectionCandidateSet { vec: vec![], ambiguous: true });
Expand Down Expand Up @@ -1383,8 +1383,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
{
debug!("assemble_candidates_for_projected_tys({:?})", obligation);

// FIXME(#20297) -- just examining the self-type is very simplistic

// before we go into the whole skolemization thing, just
// quickly check if the self-type is a projection at all.
match obligation.predicate.0.trait_ref.self_ty().sty {
Expand Down Expand Up @@ -2174,14 +2172,6 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
}

ty::TyClosure(def_id, ref substs) => {
// FIXME(#27086). We are invariant w/r/t our
// func_substs, but we don't see them as
// constituent types; this seems RIGHT but also like
// something that a normal type couldn't simulate. Is
// this just a gap with the way that PhantomData and
// OIBIT interact? That is, there is no way to say
// "make me invariant with respect to this TYPE, but
// do not act as though I can reach it"
substs.upvar_tys(def_id, self.tcx()).collect()
}

Expand Down
36 changes: 0 additions & 36 deletions src/librustc/ty/outlives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,42 +73,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
// projection).
match ty.sty {
ty::TyClosure(def_id, ref substs) => {
// FIXME(#27086). We do not accumulate from substs, since they
// don't represent reachable data. This means that, in
// practice, some of the lifetime parameters might not
// be in scope when the body runs, so long as there is
// no reachable data with that lifetime. For better or
// worse, this is consistent with fn types, however,
// which can also encapsulate data in this fashion
// (though it's somewhat harder, and typically
// requires virtual dispatch).
//
// Note that changing this (in a naive way, at least)
// causes regressions for what appears to be perfectly
// reasonable code like this:
//
// ```
// fn foo<'a>(p: &Data<'a>) {
// bar(|q: &mut Parser| q.read_addr())
// }
// fn bar(p: Box<FnMut(&mut Parser)+'static>) {
// }
// ```
//
// Note that `p` (and `'a`) are not used in the
// closure at all, but to meet the requirement that
// the closure type `C: 'static` (so it can be coerced
// to the object type), we get the requirement that
// `'a: 'static` since `'a` appears in the closure
// type `C`.
//
// A smarter fix might "prune" unused `func_substs` --
// this would avoid breaking simple examples like
// this, but would still break others (which might
// indeed be invalid, depending on your POV). Pruning
// would be a subtle process, since we have to see
// what func/type parameters are used and unused,
// taking into consideration UFCS and so forth.

for upvar_ty in substs.upvar_tys(def_id, *self) {
self.compute_components(upvar_ty, out);
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_borrowck/borrowck/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -781,8 +781,9 @@ the base path, it will still be considered freezable.



**FIXME #10520: Restrictions against mutating the base pointer.** When
an `&mut` pointer is frozen or claimed, we currently pass along the
**FIXME [RFC 1751](https://github.com/rust-lang/rfcs/issues/1751)
Restrictions against mutating the base pointer.**
When an `&mut` pointer is frozen or claimed, we currently pass along the
restriction against MUTATE to the base pointer. I do not believe this
restriction is needed. It dates from the days when we had a way to
mutate that preserved the value being mutated (i.e., swap). Nowadays
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_borrowck/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
debug!("check_if_path_is_moved(id={:?}, use_kind={:?}, lp={:?})",
id, use_kind, lp);

// FIXME (22079): if you find yourself tempted to cut and paste
Copy link
Contributor

Choose a reason for hiding this comment

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

This points to a PR, not an issue. It's fine to remove as you're doing, just pointing it out.

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 know, it's just that the FIXME is general and the number makes it seem like it is waiting on problem to be fixed.

// FIXME: if you find yourself tempted to cut and paste
// the body below and then specializing the error reporting,
// consider refactoring this instead!

Expand Down Expand Up @@ -720,7 +720,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
// the path must be initialized to prevent a case of
// partial reinitialization
//
// FIXME (22079): could refactor via hypothetical
// FIXME: could refactor via hypothetical
// generalized check_if_path_is_moved
let loan_path = owned_ptr_base_path_rc(lp_base);
self.move_data.each_move_of(id, &loan_path, |_, _| {
Expand Down
5 changes: 2 additions & 3 deletions src/librustc_borrowck/borrowck/gather_loans/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
self.all_loans.push(loan);

// if loan_gen_scope != borrow_id {
// FIXME(#6268) Nested method calls
// FIXME(https://github.com/rust-lang/rfcs/issues/811) Nested method calls
//
// Typically, the scope of the loan includes the point at
// which the loan is originated. This
Expand All @@ -417,9 +417,8 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
//let restr = restrictions::compute_restrictions(
// self.bccx, borrow_span, cmt, RESTR_EMPTY);
//let loan = {
// let all_loans = &mut *self.all_loans; // FIXME(#5074)
// Loan {
// index: all_loans.len(),
// index: self.all_loans.len(),
// loan_path,
// cmt,
// mutbl: ConstMutability,
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for RegionCtxt<'a, 'gcx, 'tcx> {
// the type of the node expr.id here *before applying
// adjustments*.
//
// FIXME(#6268) nested method calls requires that this rule change
// FIXME(https://github.com/rust-lang/rfcs/issues/811)
// nested method calls requires that this rule change
let ty0 = self.resolve_node_type(expr.hir_id);
self.type_must_outlive(infer::AddrOf(expr.span), ty0, expr_region);
intravisit::walk_expr(self, expr);
Expand Down