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

improving PR #11 #12

Merged
merged 8 commits into from
Mar 6, 2024
Merged

improving PR #11 #12

merged 8 commits into from
Mar 6, 2024

Conversation

steffahn
Copy link
Contributor

@steffahn steffahn commented Mar 6, 2024

No description provided.

steffahn added 6 commits March 6, 2024 12:27
For a few Rust versions now, sealed traits no longer need to (ab)use
public-in-private tricks, since private supertraits are allowed.

With a unsafe fn new_scope(...) -> impl Scope<...>, the type Wrapper
no longer needs to be public either.
…sed-in expressions.

Especially unsafe is important, as the macro would otherwise be unsound, allowing unsafe code
in e.g. a scop!() without explicit usage of `unsafe` from the user.
…tion

I believe rustfmt support is important enough that we should encourage this encourage
this style, and even force the inner braces to help with that. Also, this makes
the macro just take a simple $b:block, which makes the rendered docs
slightly more clear what is expected syntactically, compared to
the status quo of {$($t:tt)*}.
@steffahn
Copy link
Contributor Author

steffahn commented Mar 6, 2024

(if you merge this, consider using rebasing, or even better using fast-forward, so this becomes part of #11 cleanly without a merge commit)

…ases involving lifetimes.

For example if we have something like
```rs
fn try_zip_files<'scope, 'a: 'scope, R: Read + Seek + 'scope>(
    mut archive: ZipArchive<R>,
    s: &'a str,
) -> impl nolife::Scope<Family = ZipReaderFamily, Output = ZipResult<nolife::Never>> + 'scope {
    scope!({
        /*use s */ s;
        // ...
    }
}
```
this code fails before this commit.

(The general pattern is to add a `+ 'scope` lifetime and then
add `…: 'scope` bounds for all captured lifetimes and type parameters.)

This still leaves adding an actual test case for such a use case (and documentation for it) as TODO.
@dureuill
Copy link
Owner

dureuill commented Mar 6, 2024

Thanks, these look good. I will take the time to merge it later, hopefully by tonight.

One concern I have with #11 (not your changes) is that it is now difficult to fix all generic parameters of a BoxScope, so that the containing struct can be without generic parameters (that are related to the BoxScope). I'm not sure how to fix that, except by boxing both the dyn Scope and its Future associated type

@dureuill dureuill merged commit 0a09972 into dureuill:issue_8 Mar 6, 2024
6 of 7 checks passed
@steffahn steffahn deleted the pr_11_improvements branch March 10, 2024 16:57
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.

2 participants