mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of - Veykril:breakables, r=Veykril

Browse Source 
Properly handle break resolution inside non-breakable expressions

We now diagnose invalid `continue` expressions and properly handle things like `async` blocks which prevent labels from resolving further. Cleaned this up since `label_break_value` is on the way to stabilization in rust (🎉 finally) and we weren't handling breaks for blocks properly yet.
This commit is contained in:
bors 2022-09-01 13:07:26 +00:00
commit c4445e415a
6 changed files with 300 additions and 135 deletions
.github/workflows
ci.yaml
crates
hir-ty/src
infer.rs
infer
expr.rs
hir/src
diagnostics.rslib.rs
ide-diagnostics/src/handlers
break_outside_of_loop.rs

132
.github/workflows/ci.yaml vendored

@ -6,15 +6,15 @@ on:
pull_request:
push:
branches:
- auto
- try
- auto
- try
env:
CARGO_INCREMENTAL: 0
CARGO_NET_RETRY: 10
CI: 1
RUST_BACKTRACE: short
RUSTFLAGS: "-D warnings -W unreachable-pub -W rust-2021-compatibility"
RUSTFLAGS: "-D warnings -W unreachable-pub -W bare-trait-objects"
RUSTUP_MAX_RETRIES: 10
jobs:
@ -31,25 +31,25 @@ jobs:
os: [ubuntu-latest, windows-latest, macos-latest]
steps:
- name: Checkout repository
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 20
- name: Checkout repository
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 20
- name: Install Rust toolchain
run: |
rustup update --no-self-update stable
rustup component add rustfmt rust-src
- name: Install Rust toolchain
run: |
rustup update --no-self-update stable
rustup component add rustfmt rust-src
- name: Cache Dependencies
uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72
- name: Cache Dependencies
uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72
- name: Compile
run: cargo test --no-run --locked
- name: Compile
run: cargo test --no-run --locked
- name: Test
run: cargo test -- --nocapture --quiet
- name: Test
run: cargo test -- --nocapture --quiet
# Weird targets to catch non-portable code
rust-cross:
@ -64,25 +64,25 @@ jobs:
targets_ide: "wasm32-unknown-unknown"
steps:
- name: Checkout repository
uses: actions/checkout@v3
- name: Checkout repository
uses: actions/checkout@v3
- name: Install Rust toolchain
run: |
rustup update --no-self-update stable
rustup target add ${{ env.targets }} ${{ env.targets_ide }}
- name: Install Rust toolchain
run: |
rustup update --no-self-update stable
rustup target add ${{ env.targets }} ${{ env.targets_ide }}
- name: Cache Dependencies
uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72
- name: Cache Dependencies
uses: Swatinem/rust-cache@ce325b60658c1b38465c06cc965b79baf32c1e72
- name: Check
run: |
for target in ${{ env.targets }}; do
cargo check --target=$target --all-targets
done
for target in ${{ env.targets_ide }}; do
cargo check -p ide --target=$target --all-targets
done
- name: Check
run: |
for target in ${{ env.targets }}; do
cargo check --target=$target --all-targets
done
for target in ${{ env.targets_ide }}; do
cargo check -p ide --target=$target --all-targets
done
typescript:
if: github.repository == 'rust-lang/rust-analyzer'
@ -95,47 +95,47 @@ jobs:
runs-on: ${{ matrix.os }}
steps:
- name: Checkout repository
uses: actions/checkout@v3
- name: Checkout repository
uses: actions/checkout@v3
- name: Install Nodejs
uses: actions/setup-node@v1
with:
node-version: 16.x
- name: Install Nodejs
uses: actions/setup-node@v1
with:
node-version: 16.x
- name: Install xvfb
if: matrix.os == 'ubuntu-latest'
run: sudo apt-get install -y xvfb
- name: Install xvfb
if: matrix.os == 'ubuntu-latest'
run: sudo apt-get install -y xvfb
- run: npm ci
working-directory: ./editors/code
- run: npm ci
working-directory: ./editors/code
# - run: npm audit || { sleep 10 && npm audit; } || { sleep 30 && npm audit; }
# if: runner.os == 'Linux'
# working-directory: ./editors/code
# - run: npm audit || { sleep 10 && npm audit; } || { sleep 30 && npm audit; }
# if: runner.os == 'Linux'
# working-directory: ./editors/code
- run: npm run lint
working-directory: ./editors/code
- run: npm run lint
working-directory: ./editors/code
- name: Run VS Code tests (Linux)
if: matrix.os == 'ubuntu-latest'
env:
VSCODE_CLI: 1
run: xvfb-run npm test
working-directory: ./editors/code
- name: Run VS Code tests (Linux)
if: matrix.os == 'ubuntu-latest'
env:
VSCODE_CLI: 1
run: xvfb-run npm test
working-directory: ./editors/code
- name: Run VS Code tests (Windows)
if: matrix.os == 'windows-latest'
env:
VSCODE_CLI: 1
run: npm test
working-directory: ./editors/code
- name: Run VS Code tests (Windows)
if: matrix.os == 'windows-latest'
env:
VSCODE_CLI: 1
run: npm test
working-directory: ./editors/code
- run: npm run pretest
working-directory: ./editors/code
- run: npm run pretest
working-directory: ./editors/code
- run: npm run package --scripts-prepend-node-path
working-directory: ./editors/code
- run: npm run package --scripts-prepend-node-path
working-directory: ./editors/code
end-success:
name: bors build finished

33
crates/hir-ty/src/infer.rs

@ -182,7 +182,7 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, TypeError>;
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum InferenceDiagnostic {
NoSuchField { expr: ExprId },
BreakOutsideOfLoop { expr: ExprId },
BreakOutsideOfLoop { expr: ExprId, is_break: bool },
MismatchedArgCount { call_expr: ExprId, expected: usize, found: usize },
}
@ -418,18 +418,45 @@ pub(crate) struct InferenceContext<'a> {
#[derive(Clone, Debug)]
struct BreakableContext {
/// Whether this context contains at least one break expression.
may_break: bool,
/// The coercion target of the context.
coerce: CoerceMany,
/// The optional label of the context.
label: Option<name::Name>,
kind: BreakableKind,
}
#[derive(Clone, Debug)]
enum BreakableKind {
Block,
Loop,
/// A border is something like an async block, closure etc. Anything that prevents
/// breaking/continuing through
Border,
}
fn find_breakable<'c>(
ctxs: &'c mut [BreakableContext],
label: Option<&name::Name>,
) -> Option<&'c mut BreakableContext> {
let mut ctxs = ctxs
.iter_mut()
.rev()
.take_while(|it| matches!(it.kind, BreakableKind::Block | BreakableKind::Loop));
match label {
Some(_) => ctxs.iter_mut().rev().find(|ctx| ctx.label.as_ref() == label),
None => ctxs.last_mut(),
Some(_) => ctxs.find(|ctx| ctx.label.as_ref() == label),
None => ctxs.find(|ctx| matches!(ctx.kind, BreakableKind::Loop)),
}
}
fn find_continuable<'c>(
ctxs: &'c mut [BreakableContext],
label: Option<&name::Name>,
) -> Option<&'c mut BreakableContext> {
match label {
Some(_) => find_breakable(ctxs, label).filter(|it| matches!(it.kind, BreakableKind::Loop)),
None => find_breakable(ctxs, label),
}
}

143
crates/hir-ty/src/infer/expr.rs

@ -10,7 +10,7 @@ use chalk_ir::{
cast::Cast, fold::Shift, DebruijnIndex, GenericArgData, Mutability, TyVariableKind,
};
use hir_def::{
expr::{ArithOp, Array, BinaryOp, CmpOp, Expr, ExprId, Literal, Statement, UnaryOp},
expr::{ArithOp, Array, BinaryOp, CmpOp, Expr, ExprId, LabelId, Literal, Statement, UnaryOp},
generics::TypeOrConstParamData,
path::{GenericArg, GenericArgs},
resolver::resolver_for_expr,
@ -23,7 +23,7 @@ use syntax::ast::RangeOp;
use crate::{
autoderef::{self, Autoderef},
consteval,
infer::coerce::CoerceMany,
infer::{coerce::CoerceMany, find_continuable, BreakableKind},
lower::{
const_or_path_to_chalk, generic_arg_to_chalk, lower_to_chalk_mutability, ParamLoweringMode,
},
@ -120,32 +120,37 @@ impl<'a> InferenceContext<'a> {
let ty = match label {
Some(_) => {
let break_ty = self.table.new_type_var();
self.breakables.push(BreakableContext {
may_break: false,
coerce: CoerceMany::new(break_ty.clone()),
label: label.map(|label| self.body[label].name.clone()),
});
let ty = self.infer_block(
tgt_expr,
statements,
*tail,
&Expectation::has_type(break_ty),
let (breaks, ty) = self.with_breakable_ctx(
BreakableKind::Block,
break_ty.clone(),
*label,
|this| {
this.infer_block(
tgt_expr,
statements,
*tail,
&Expectation::has_type(break_ty),
)
},
);
let ctxt = self.breakables.pop().expect("breakable stack broken");
if ctxt.may_break {
ctxt.coerce.complete()
} else {
ty
}
breaks.unwrap_or(ty)
}
None => self.infer_block(tgt_expr, statements, *tail, expected),
};
self.resolver = old_resolver;
ty
}
Expr::Unsafe { body } | Expr::Const { body } => self.infer_expr(*body, expected),
Expr::Unsafe { body } => self.infer_expr(*body, expected),
Expr::Const { body } => {
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
this.infer_expr(*body, expected)
})
.1
}
Expr::TryBlock { body } => {
let _inner = self.infer_expr(*body, expected);
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
let _inner = this.infer_expr(*body, expected);
});
// FIXME should be std::result::Result<{inner}, _>
self.err_ty()
}
@ -154,7 +159,10 @@ impl<'a> InferenceContext<'a> {
let prev_diverges = mem::replace(&mut self.diverges, Diverges::Maybe);
let prev_ret_ty = mem::replace(&mut self.return_ty, ret_ty.clone());
let inner_ty = self.infer_expr_coerce(*body, &Expectation::has_type(ret_ty));
let (_, inner_ty) =
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
this.infer_expr_coerce(*body, &Expectation::has_type(ret_ty))
});
self.diverges = prev_diverges;
self.return_ty = prev_ret_ty;
@ -166,54 +174,44 @@ impl<'a> InferenceContext<'a> {
TyKind::OpaqueType(opaque_ty_id, Substitution::from1(Interner, inner_ty))
.intern(Interner)
}
Expr::Loop { body, label } => {
self.breakables.push(BreakableContext {
may_break: false,
coerce: CoerceMany::new(self.table.new_type_var()),
label: label.map(|label| self.body[label].name.clone()),
});
self.infer_expr(*body, &Expectation::has_type(TyBuilder::unit()));
&Expr::Loop { body, label } => {
let ty = self.table.new_type_var();
let (breaks, ()) =
self.with_breakable_ctx(BreakableKind::Loop, ty, label, |this| {
this.infer_expr(body, &Expectation::has_type(TyBuilder::unit()));
});
let ctxt = self.breakables.pop().expect("breakable stack broken");
if ctxt.may_break {
self.diverges = Diverges::Maybe;
ctxt.coerce.complete()
} else {
TyKind::Never.intern(Interner)
match breaks {
Some(breaks) => {
self.diverges = Diverges::Maybe;
breaks
}
None => TyKind::Never.intern(Interner),
}
}
Expr::While { condition, body, label } => {
self.breakables.push(BreakableContext {
may_break: false,
coerce: CoerceMany::new(self.err_ty()),
label: label.map(|label| self.body[label].name.clone()),
&Expr::While { condition, body, label } => {
self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| {
this.infer_expr(
condition,
&Expectation::has_type(TyKind::Scalar(Scalar::Bool).intern(Interner)),
);
this.infer_expr(body, &Expectation::has_type(TyBuilder::unit()));
});
self.infer_expr(
*condition,
&Expectation::has_type(TyKind::Scalar(Scalar::Bool).intern(Interner)),
);
self.infer_expr(*body, &Expectation::has_type(TyBuilder::unit()));
let _ctxt = self.breakables.pop().expect("breakable stack broken");
// the body may not run, so it diverging doesn't mean we diverge
self.diverges = Diverges::Maybe;
TyBuilder::unit()
}
Expr::For { iterable, body, pat, label } => {
let iterable_ty = self.infer_expr(*iterable, &Expectation::none());
self.breakables.push(BreakableContext {
may_break: false,
coerce: CoerceMany::new(self.err_ty()),
label: label.map(|label| self.body[label].name.clone()),
});
&Expr::For { iterable, body, pat, label } => {
let iterable_ty = self.infer_expr(iterable, &Expectation::none());
let pat_ty =
self.resolve_associated_type(iterable_ty, self.resolve_into_iter_item());
self.infer_pat(*pat, &pat_ty, BindingMode::default());
self.infer_pat(pat, &pat_ty, BindingMode::default());
self.with_breakable_ctx(BreakableKind::Loop, self.err_ty(), label, |this| {
this.infer_expr(body, &Expectation::has_type(TyBuilder::unit()));
});
self.infer_expr(*body, &Expectation::has_type(TyBuilder::unit()));
let _ctxt = self.breakables.pop().expect("breakable stack broken");
// the body may not run, so it diverging doesn't mean we diverge
self.diverges = Diverges::Maybe;
TyBuilder::unit()
@ -269,7 +267,9 @@ impl<'a> InferenceContext<'a> {
let prev_diverges = mem::replace(&mut self.diverges, Diverges::Maybe);
let prev_ret_ty = mem::replace(&mut self.return_ty, ret_ty.clone());
self.infer_expr_coerce(*body, &Expectation::has_type(ret_ty));
self.with_breakable_ctx(BreakableKind::Border, self.err_ty(), None, |this| {
this.infer_expr_coerce(*body, &Expectation::has_type(ret_ty));
});
self.diverges = prev_diverges;
self.return_ty = prev_ret_ty;
@ -372,7 +372,15 @@ impl<'a> InferenceContext<'a> {
let resolver = resolver_for_expr(self.db.upcast(), self.owner, tgt_expr);
self.infer_path(&resolver, p, tgt_expr.into()).unwrap_or_else(|| self.err_ty())
}
Expr::Continue { .. } => TyKind::Never.intern(Interner),
Expr::Continue { label } => {
if let None = find_continuable(&mut self.breakables, label.as_ref()) {
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
expr: tgt_expr,
is_break: false,
});
};
TyKind::Never.intern(Interner)
}
Expr::Break { expr, label } => {
let mut coerce = match find_breakable(&mut self.breakables, label.as_ref()) {
Some(ctxt) => {
@ -400,6 +408,7 @@ impl<'a> InferenceContext<'a> {
} else {
self.push_diagnostic(InferenceDiagnostic::BreakOutsideOfLoop {
expr: tgt_expr,
is_break: true,
});
};
@ -1472,4 +1481,20 @@ impl<'a> InferenceContext<'a> {
},
})
}
fn with_breakable_ctx<T>(
&mut self,
kind: BreakableKind,
ty: Ty,
label: Option<LabelId>,
cb: impl FnOnce(&mut Self) -> T,
) -> (Option<Ty>, T) {
self.breakables.push({
let label = label.map(|label| self.body[label].name.clone());
BreakableContext { kind, may_break: false, coerce: CoerceMany::new(ty), label }
});
let res = cb(self);
let ctx = self.breakables.pop().expect("breakable stack broken");
(ctx.may_break.then(|| ctx.coerce.complete()), res)
}
}

1
crates/hir/src/diagnostics.rs

@ -124,6 +124,7 @@ pub struct NoSuchField {
#[derive(Debug)]
pub struct BreakOutsideOfLoop {
pub expr: InFile<AstPtr<ast::Expr>>,
pub is_break: bool,
}
#[derive(Debug)]

6
crates/hir/src/lib.rs

@ -1216,11 +1216,11 @@ impl DefWithBody {
let field = source_map.field_syntax(*expr);
acc.push(NoSuchField { field }.into())
}
hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { expr } => {
&hir_ty::InferenceDiagnostic::BreakOutsideOfLoop { expr, is_break } => {
let expr = source_map
.expr_syntax(*expr)
.expr_syntax(expr)
.expect("break outside of loop in synthetic syntax");
acc.push(BreakOutsideOfLoop { expr }.into())
acc.push(BreakOutsideOfLoop { expr, is_break }.into())
}
hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => {
match source_map.expr_syntax(*call_expr) {

120
crates/ide-diagnostics/src/handlers/break_outside_of_loop.rs

@ -7,9 +7,10 @@ pub(crate) fn break_outside_of_loop(
ctx: &DiagnosticsContext<'_>,
d: &hir::BreakOutsideOfLoop,
) -> Diagnostic {
let construct = if d.is_break { "break" } else { "continue" };
Diagnostic::new(
"break-outside-of-loop",
"break outside of loop",
format!("{construct} outside of loop"),
ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range,
)
}
@ -19,11 +20,122 @@ mod tests {
use crate::tests::check_diagnostics;
#[test]
fn break_outside_of_loop() {
fn outside_of_loop() {
check_diagnostics(
r#"
fn foo() { break; }
//^^^^^ error: break outside of loop
fn foo() {
break;
//^^^^^ error: break outside of loop
break 'a;
//^^^^^^^^ error: break outside of loop
continue;
//^^^^^^^^ error: continue outside of loop
continue 'a;
//^^^^^^^^^^^ error: continue outside of loop
}
"#,
);
}
#[test]
fn try_blocks_are_borders() {
check_diagnostics(
r#"
fn foo() {
'a: loop {
try {
break;
//^^^^^ error: break outside of loop
break 'a;
//^^^^^^^^ error: break outside of loop
continue;
//^^^^^^^^ error: continue outside of loop
continue 'a;
//^^^^^^^^^^^ error: continue outside of loop
};
}
}
"#,
);
}
#[test]
fn async_blocks_are_borders() {
check_diagnostics(
r#"
fn foo() {
'a: loop {
try {
break;
//^^^^^ error: break outside of loop
break 'a;
//^^^^^^^^ error: break outside of loop
continue;
//^^^^^^^^ error: continue outside of loop
continue 'a;
//^^^^^^^^^^^ error: continue outside of loop
};
}
}
"#,
);
}
#[test]
fn closures_are_borders() {
check_diagnostics(
r#"
fn foo() {
'a: loop {
try {
break;
//^^^^^ error: break outside of loop
break 'a;
//^^^^^^^^ error: break outside of loop
continue;
//^^^^^^^^ error: continue outside of loop
continue 'a;
//^^^^^^^^^^^ error: continue outside of loop
};
}
}
"#,
);
}
#[test]
fn blocks_pass_through() {
check_diagnostics(
r#"
fn foo() {
'a: loop {
{
break;
break 'a;
continue;
continue 'a;
}
}
}
"#,
);
}
#[test]
fn label_blocks() {
check_diagnostics(
r#"
fn foo() {
'a: {
break;
//^^^^^ error: break outside of loop
break 'a;
continue;
//^^^^^^^^ error: continue outside of loop
continue 'a;
//^^^^^^^^^^^ error: continue outside of loop
}
}
"#,
);
}