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

Suggest using an inclusive range instead of an exclusive range when the endpoint overflows by 1 #60330

Merged
merged 9 commits into from
May 1, 2019
62 changes: 62 additions & 0 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5400,3 +5400,65 @@ fn body_ids(bodies: &BTreeMap<hir::BodyId, hir::Body>) -> Vec<hir::BodyId> {
body_ids.sort_by_key(|b| bodies[b].value.span);
body_ids
}

/// Checks if the specified expression is a built-in range literal.
/// (See: `LoweringContext::lower_expr()`).
pub fn is_range_literal(sess: &Session, expr: &hir::Expr) -> bool {
use hir::{Path, QPath, ExprKind, TyKind};

// Returns whether the given path represents a (desugared) range,
// either in std or core, i.e. has either a `::std::ops::Range` or
// `::core::ops::Range` prefix.
fn is_range_path(path: &Path) -> bool {
let segs: Vec<_> = path.segments.iter().map(|seg| seg.ident.as_str().to_string()).collect();
let segs: Vec<_> = segs.iter().map(|seg| &**seg).collect();

// "{{root}}" is the equivalent of `::` prefix in `Path`.
if let ["{{root}}", std_core, "ops", range] = segs.as_slice() {
(*std_core == "std" || *std_core == "core") && range.starts_with("Range")
} else {
false
}
};

// Check whether a span corresponding to a range expression is a
// range literal, rather than an explicit struct or `new()` call.
fn is_lit(sess: &Session, span: &Span) -> bool {
let source_map = sess.source_map();
let end_point = source_map.end_point(*span);

if let Ok(end_string) = source_map.span_to_snippet(end_point) {
!(end_string.ends_with("}") || end_string.ends_with(")"))
} else {
false
}
};

match expr.node {
// All built-in range literals but `..=` and `..` desugar to `Struct`s.
ExprKind::Struct(ref qpath, _, _) => {
if let QPath::Resolved(None, ref path) = **qpath {
return is_range_path(&path) && is_lit(sess, &expr.span);
}
}

// `..` desugars to its struct path.
ExprKind::Path(QPath::Resolved(None, ref path)) => {
return is_range_path(&path) && is_lit(sess, &expr.span);
}

// `..=` desugars into `::std::ops::RangeInclusive::new(...)`.
ExprKind::Call(ref func, _) => {
if let ExprKind::Path(QPath::TypeRelative(ref ty, ref segment)) = func.node {
if let TyKind::Path(QPath::Resolved(None, ref path)) = ty.node {
let new_call = segment.ident.as_str() == "new";
return is_range_path(&path) && is_lit(sess, &expr.span) && new_call;
}
}
}

_ => {}
}

false
}
Loading