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

Hygienic bug in macro #16

Closed
TeXitoi opened this issue Jul 30, 2021 · 4 comments
Closed

Hygienic bug in macro #16

TeXitoi opened this issue Jul 30, 2021 · 4 comments

Comments

@TeXitoi
Copy link

TeXitoi commented Jul 30, 2021

self_cell doesn't allow a custom result export:

use self_cell::self_cell;

// Remove this, and it compiles
type Result<T> = std::result::Result<T, ()>;

#[derive(Debug, Eq, PartialEq)]
struct Ast<'a>(pub Vec<&'a str>);

self_cell!(
    struct AstCell {
        owner: String,

        #[covariant]
        dependent: Ast,
    }

    impl {Debug, Eq, PartialEq}
);

fn build_ast_cell(code: &str) -> AstCell {
    // Create owning String on stack.
    let pre_processed_code = code.trim().to_string();

    // Move String into AstCell, then build Ast inplace.
    AstCell::new(pre_processed_code, |code| {
        Ast(code.split(' ').filter(|word| word.len() > 1).collect())
    })
}

fn main() {
    let ast_cell = build_ast_cell("fox = cat + dog");

    println!("ast_cell -> {:?}", &ast_cell);
    println!("ast_cell.borrow_owner() -> {:?}", ast_cell.borrow_owner());
    println!(
        "ast_cell.borrow_dependent().0[1] -> {:?}",
        ast_cell.borrow_dependent().0[1]
    );
}
@Voultapher
Copy link
Owner

Thanks for pointing that out, I just pushed 34acff4 please let me know if that addresses your issue.

@TeXitoi
Copy link
Author

TeXitoi commented Aug 3, 2021

Yes, it should do the trick.

Voultapher added a commit that referenced this issue Aug 3, 2021
This release brings 2 core changes:

- `panic` inside a builder function no longer causes a memory leak.

This means that the previously documented workaround using `try_new` and
`catch_unwind` has become obsolete and the relevant example has been deleted.

- Limited support for owners with non static lifetimes. This mostly works with the exception of automatic derives, due to technical limitations (aka I can't figure out how to do it).

Bug fixes:

- An esoteric edge case could cause UB, see bb2f502 for more details.

- Defining your own Result could break the macro due to hygiene issues, see #16 (comment).

In addition a set of benchmarks comparing `self_cell` to `ouroboros` have been added, see `benchmarks` directory.
@Voultapher
Copy link
Owner

I just released a new version which includes this fix https://github.com/Voultapher/self_cell/releases/tag/v0.9.1

@TeXitoi
Copy link
Author

TeXitoi commented Aug 3, 2021

Thanks

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

No branches or pull requests

2 participants