Skip to content

Conversation

@ubnt-intrepid
Copy link
Contributor

It changes the desugaring rule to bind the return value to a temporary variable before wrapping, rather than modifying the AST of tail expression.

This PR is a successor of #43 and the expansion rule comes originally from #43 (comment).

It fixes #42 at the same time since the wrapping expression always suppresses the warning message by #[allow(unreachable_code)].

It changes the desugaring rule to bind the return value to a
temporary variable before wrapping, rather than modifying the AST
of tail expression.
@withoutboats
Copy link
Owner

Haha and honestly the code is much cleaner than what it was doing before! I'll test it out on some codebases locally to check that it works well and get back to you. Thanks!

@withoutboats
Copy link
Owner

So, this is a breaking change. But it's one that probably will strictly improve things, so I'm planning that we merge it and release a 2.0 at some point.

I found two breakages in my code that result from this.

The first is that the current code will allow some code to compile that really it shouldn't. In particular, consider this:

#[throws(())]
fn foo() {
    0
}

This code should not compile, because the final expression doesn't have the same type as the return type. Unfortunately, on the 1.0.0 branch it does, because we just insert a Ok(()) after it. In your branch, it correctly does not compile.

The second error I found arose from using throws in a trait definition. Now, I believe this causes the body to be expanded to let _ret = { ; }; .... So if the trait method was returning something, now you get a type error, when really you shouldn't. This is connected to the problem that #40 is aiming at: we don't really handle trait methods very well, and there are bugs around that. We should not be modifying the bodies of trait methods without bodies at all.

I'm going to merge this now, and then hopefully through #40 and maybe some subsequent PRs if necessary we can get it handling trait methods properly.

@withoutboats withoutboats merged commit df2e3d4 into withoutboats:master Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unreachable expression warning in tail throw

2 participants