Skip to content

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Apr 1, 2020

Closes #70490
Closes #56828

r? @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2020
@lcnr lcnr changed the title try-blocks: don't lint required delims try-blocks: don't emit unused delims lint for required delims Apr 1, 2020
@lcnr lcnr force-pushed the unused_delims_try branch from 01b8530 to b3f9ed0 Compare April 1, 2020 14:47
@@ -371,7 +371,7 @@ trait UnusedDelimLint {
fn is_expr_delims_necessary(inner: &ast::Expr, followed_by_block: bool) -> bool {
followed_by_block
&& match inner.kind {
ast::ExprKind::Ret(_) | ast::ExprKind::Break(..) => true,
ExprKind::Ret(_) | ExprKind::Break(..) | ExprKind::TryBlock(_) => true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like a parser bug to me. We have:

    fn is_try_block(&self) -> bool {
        self.token.is_keyword(kw::Try) &&
        self.look_ahead(1, |t| *t == token::OpenDelim(token::Brace)) &&
        self.token.uninterpolated_span().rust_2018() &&
        // Prevent `while try {} {}`, `if try {} {} else {}`, etc.
        !self.restrictions.contains(Restrictions::NO_STRUCT_LITERAL)
    }

but we allow this for unsafe {}, async {}, etc.

cc @petrochenkov

@joelpalmer joelpalmer added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 14, 2020
@Dylan-DPC-zz

This comment has been minimized.

@lcnr lcnr force-pushed the unused_delims_try branch from b3f9ed0 to b9106c1 Compare April 14, 2020 15:45
@lcnr
Copy link
Contributor Author

lcnr commented Apr 14, 2020

Waited for a comment by @petrochenkov on this, as this changes the purpose of this PR.

I changed the parser to allow try in scrutinee position/in places where open braces are expected.

// This now compiles
match try {} {
    Ok(()) | Err(()) => (),
}

@Centril Centril added the F-try_blocks `#![feature(try_blocks)]` label Apr 14, 2020
@lcnr lcnr changed the title try-blocks: don't emit unused delims lint for required delims Allow try-blocks in places where an open delim is expected Apr 14, 2020
@lcnr lcnr force-pushed the unused_delims_try branch from b9106c1 to 304513c Compare April 14, 2020 15:51
@Centril
Copy link
Contributor

Centril commented Apr 14, 2020

Did you intend to include that submodule bump?

@lcnr
Copy link
Contributor Author

lcnr commented Apr 14, 2020

I rarely do 😆 thank you for catching this

@lcnr lcnr force-pushed the unused_delims_try branch 2 times, most recently from 4010c1d to f74e466 Compare April 14, 2020 15:59
@Centril
Copy link
Contributor

Centril commented Apr 14, 2020

cc @scottmcm, this fixes #56828 which we agreed was a bug at the time.

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 14, 2020

📌 Commit f74e466771ad79410b84b2034d45cc3cc1d84855 has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 14, 2020
@lcnr lcnr force-pushed the unused_delims_try branch from f74e466 to 81a3cd7 Compare April 14, 2020 16:39
@Centril
Copy link
Contributor

Centril commented Apr 14, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 14, 2020

📌 Commit 81a3cd7 has been approved by Centril

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2020
Allow `try`-blocks in places where an open delim is expected

Closes rust-lang#70490
Closes rust-lang#56828

r? @Centril
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2020
Rollup of 8 pull requests

Successful merges:

 - rust-lang#70657 (Allow `try`-blocks in places where an open delim is expected)
 - rust-lang#70947 (tighten CTFE safety net for accesses to globals)
 - rust-lang#70949 (simplify `vec!` macro)
 - rust-lang#71002 (fix target & runtool args order)
 - rust-lang#71082 (ptr: introduce len() method on raw slices)
 - rust-lang#71128 (Remove unused single_step flag)
 - rust-lang#71133 (Tighten time complexity on the doc of sort_by_key)
 - rust-lang#71135 (Update books)

Failed merges:

r? @ghost
@bors bors merged commit 15ab586 into rust-lang:master Apr 15, 2020
@lcnr lcnr deleted the unused_delims_try branch April 15, 2020 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-try_blocks `#![feature(try_blocks)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unused_parens incorrectly lints on if let Ok(x) = (try { ... try_block cannot be used everywhere an expression can
6 participants