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

Make a changed(somevar) macro for loops #211

Open
masak opened this issue Jan 13, 2017 · 6 comments
Open

Make a changed(somevar) macro for loops #211

masak opened this issue Jan 13, 2017 · 6 comments

Comments

@masak
Copy link
Owner

masak commented Jan 13, 2017

I'm thinking of a loop like this:

my dataStructure = ...;
my stillChanging = True;
while stillChanging {
    stillChanging = False;
    # ...logic...
    if ... {
        dataStructure = ...;
        stillChanging = True;
    }
}

This proposed macro would allow us to hide the stillChanging boolean and have the macro generate the code to update it properly:

my dataStructure = ...;
while changed(dataStructure) {
    # ...logic...
    if ... {
        dataStructure = ...;
    }
}
@masak
Copy link
Owner Author

masak commented Feb 25, 2018

A related macro deeplyChanged could detect changes not just to the variable itself (which is still useful and exactly what we need sometimes), but to nested accesses to the variable.

@masak
Copy link
Owner Author

masak commented Feb 9, 2019

I wonder if #186 could be given a semantics such that this issue can just use it straight off?

@masak
Copy link
Owner Author

masak commented Jun 8, 2019

Just driving by to say I think the while in OP should have been a repeat while, especially so since I'd intuitively expect changed(X) for any X to be false — with no previous value to compare against, we can't reasonably claim X has changed.

@masak
Copy link
Owner Author

masak commented Oct 13, 2021

Much later thought: this feature might be trying to be "too magic" by implicitly deciding when something changed (and injecting code at that point). With variables, it's easy to detect a change, just look for =. But with arrays, you'll need to go looking for push calls and the like — ditto for all other data containers, including some future ones that the macro can not know about. The fact that it's playing catch-up with all forms of modification turns into a kind of brittleness, and the risk of false negatives.

Maybe better to have some convenient syntax when something did change. (Not committing on what.) Then the macro up in the loop could just look for these explicit signals, and the programmer could make sure to remember to explicitly mark the places that change.

@masak
Copy link
Owner Author

masak commented May 1, 2023

Still much later thought: I actually wrote a loop like this in TypeScript. Let me start by pasting the whole function, showing the loop in its context:

export function solve(system: EqnSystem): Array<boolean> {
    function refsConst(bvar: BoolExp): boolean {
        if (!(bvar instanceof BoolExp.BVar)) {
            return false;
        }
        let eqn = system.eqns.find((e) => e.lhs === bvar.name);
        if (eqn === undefined) {
            throw new Error(
                "Reference to unknown equation -- precondition broken");
        }
        return eqn.rhs instanceof BoolExp.BConst;
    }

    let changed: boolean;
    do {
        changed = false;
        for (let eqn of system.eqns) {
            let rhs = eqn.rhs;
            // simplification rules
            if (rhs instanceof BoolExp.All && rhs.operands.some(isFalse)) {
                eqn.rhs = new BoolExp.BConst(false);
                changed = true;
            }
            else if (rhs instanceof BoolExp.Any && rhs.operands.some(isTrue)) {
                eqn.rhs = new BoolExp.BConst(true);
                changed = true;
            }
            else if (rhs instanceof BoolExp.All && rhs.operands.length === 1) {
                eqn.rhs = rhs.operands[0];
                changed = true;
            }
            else if (rhs instanceof BoolExp.Any && rhs.operands.length === 1) {
                eqn.rhs = rhs.operands[0];
                changed = true;
            }
            else if (rhs instanceof BoolExp.All && rhs.operands.length === 0) {
                eqn.rhs = new BoolExp.BConst(true);
                changed = true;
            }
            else if (rhs instanceof BoolExp.Any && rhs.operands.length === 0) {
                eqn.rhs = new BoolExp.BConst(false);
                changed = true;
            }
            if (rhs instanceof BoolExp.All && rhs.operands.every(isTrue)) {
                eqn.rhs = new BoolExp.BConst(true);
                changed = true;
            }
            else if (rhs instanceof BoolExp.Any &&
                        rhs.operands.every(isFalse)) {
                eqn.rhs = new BoolExp.BConst(false);
                changed = true;
            }
            else if (rhs instanceof BoolExp.BVar && refsConst(rhs)) {
                let otherEqn = system.eqns.find(
                    (e) => e.lhs === (rhs as BoolExp.BVar).name)!;
                eqn.rhs = otherEqn.rhs;
                changed = true;
            }
            else if (rhs instanceof BoolExp.All &&
                        rhs.operands.some(refsConst)) {
                rhs.operands = rhs.operands.map((exp) => {
                    if (refsConst(exp)) {
                        let otherEqn = system.eqns.find(
                            (e) => e.lhs === (exp as BoolExp.BVar).name)!;
                        return otherEqn.rhs;
                    }
                    else {
                        return exp;
                    }
                });
                changed = true;
            }
            else if (rhs instanceof BoolExp.Any &&
                        rhs.operands.some(refsConst)) {
                rhs.operands = rhs.operands.map((exp) => {
                    if (refsConst(exp)) {
                        let otherEqn = system.eqns.find(
                            (e) => e.lhs === (exp as BoolExp.BVar).name)!;
                        return otherEqn.rhs;
                    }
                    else {
                        return exp;
                    }
                });
                changed = true;
            }
        }
    } while (changed);

    return system.eqns.map((eqn) => {
        let rhs = eqn.rhs;
        return rhs instanceof BoolExp.BConst
            ? rhs.value
            : system.cycleDefault;
    });
}

Three points:

  • The entire code "slice" having to do with changed does indeed strongly feel like boilerplate.
  • When I wrote this code, I was worried I might miss one changed = true; statement somewhere. I didn't, but I was still worried. That's because such worry is legitimate. That's because it's boilerplate, and I'm wasting developer cycles maintaining the boilerplate. (Which is a long way of saying, I still strongly believe there should be a macro here.)
  • What we are monitoring in this case is any change to (the contents of) eqn. Most of the branches do indeed change eqn.rhs, and we could detect that easily. But the last two change rhs.operands, which (via assignment to rhs) is identical to eqn.rhs.operands — the point is that a good macro would need to track that; that is, track assigned variables and count any changes via them, too. On the other hand, this remains a strictly "local" analysis.

@masak
Copy link
Owner Author

masak commented Aug 10, 2023

Just driving by to say I think the while in OP should have been a repeat while, especially so since I'd intuitively expect changed(X) for any X to be false — with no previous value to compare against, we can't reasonably claim X has changed.

I drove by to say this, which was already said.

So I'll say something else instead. Yes, while this is a strictly "local" analysis, it also feels like it'd have to be a contextual macro, in the sense that changed(X) wants to work on much more than its argument X. It wants to work on... all occurrences of assignments to X found inside the loop body, and even inject a fresh changed variable right above the loop. So it needs access to all of that. Don't get me wrong, I think that would be great. But it needs that.

There's also the slight worry of "how do we know something is an assignment?" — which sounds silly at first, until we realize that anyone can extend the syntax and semantics, and so there isn't a priori a way to ask the question that isn't disrupted by such extensions. Presumably something like an "assignment protocol", or a way to query the control-flow-graph rather than the AST, could be made to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant