ekzhang / crepe

Datalog compiler embedded in Rust as a procedural macro
Apache License 2.0
453 stars 16 forks source link

For expressions unexpectedly introduce new variables #22

Closed Cypher1 closed 2 years ago

Cypher1 commented 2 years ago

Don't have a minimal repro, but I found that the new for expression added in #17 can introduce shadowing variable names. I think ideally these expressions would restrict by the already bound variables, but recognizing these new bindings and throwing an error would be a great step forward.

Thanks!

ekzhang commented 2 years ago

Unfortunately I'm not sure what you mean. Variables shadowing previous declarations is normal and expected behavior inherited from Rust. Can you provide an example?

Cypher1 commented 2 years ago

Sure thing. Sorry I rushed creating the bug, needed to get out of the office.

I'll write up a minimal repro Monday so you can decide if this is working as intended.

Thanks

On Sun, 6 Mar 2022, 2:07 pm Eric Zhang, @.***> wrote:

Unfortunately I'm not sure what you mean. Variables shadowing previous declarations is normal and expected behavior inherited from Rust. Can you provide an example?

— Reply to this email directly, view it on GitHub https://github.com/ekzhang/crepe/issues/22#issuecomment-1059885153, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIRUHXXCA4SFS473IRBT6LU6QOQVANCNFSM5P4LIO6A . You are receiving this because you authored the thread.Message ID: @.***>

Cypher1 commented 2 years ago

I think this is a neat repo:

A reachability solver:

fn edges() -> Vec<Edge> {
    vec![
        Edge(1, 2),
        Edge(3, 4),
    ]
}

use crepe::crepe;

crepe!{
    @input
    struct Start(i32); // start

    @input
    struct Edge(i32, i32);

    @output
    #[derive(Debug)]
    struct Reachable(i32, i32); // start, to

    Reachable(s, s) <- Start(s);
    Reachable(s, t) <- Reachable(s, f), Edge(f, t);
}

fn main() {
    let mut runtime = Crepe::new();
    runtime.extend(vec![Start(3)]);
    runtime.extend(edges());
    let (sol,) = runtime.run();
    dbg!(sol);
}

Results in

[src/main.rs:30] sol = {
    Reachable(
        3,
        3,
    ),
    Reachable(
        3,
        4,
    ),
}

But implementing this naively using a for loop:

struct Edge(i32, i32);

fn edges() -> Vec<Edge> {
    vec![
        Edge(1, 2),
        Edge(3, 4),
    ]
}

use crepe::crepe;

crepe!{
    @input
    struct Start(i32); // start

    @output
    #[derive(Debug)]
    struct Reachable(i32, i32); // start, to

    Reachable(s, s) <- Start(s);
    Reachable(s, t) <- Reachable(s, f), for Edge(f, t) in edges();
}

fn main() {
    let mut runtime = Crepe::new();
    runtime.extend(vec![Start(3)]);
    let (sol,) = runtime.run();
    dbg!(sol);
}

Incorrectly gives

[src/main.rs:28] sol = {
    Reachable(
        3,
        2,
    ),
    Reachable(
        3,
        3,
    ),
    Reachable(
        3,
        4,
    ),
}

Patching the program requires a new variable name and another check

struct Edge(i32, i32);

fn edges() -> Vec<Edge> {
    vec![
        Edge(1, 2),
        Edge(3, 4),
    ]
}

use crepe::crepe;

crepe!{
    @input
    struct Start(i32); // start

    @output
    #[derive(Debug)]
    struct Reachable(i32, i32); // start, to

    Reachable(s, s) <- Start(s);
    Reachable(s, t) <- Reachable(s, f), for Edge(up, t) in edges(), (up == f);
}

fn main() {
    let mut runtime = Crepe::new();
    runtime.extend(vec![Start(3)]);
    let (sol,) = runtime.run();
    dbg!(sol);
}

Hope this clears it up

ekzhang commented 2 years ago

Ah, I see why that might possibly be confusing for a new user. You would probably want to either change your function to get all the edges from a given vertex f, or you could also reorder the clauses to avoid the issue.

I don't think there's a better alternative to this. The behavior is working as intended and is consistent with Rust's variable shadowing semantics. Thank you for the example though!