search

purpleidea / mgmt

Next generation distributed, event-driven, parallel config management!
https://purpleidea.com/tags/mgmtconfig/
GNU General Public License v3.0
3.58k stars 311 forks source link

lang: Remove Expr::SetValue() #695

Closed gelisam closed 1 year ago

gelisam commented 1 year ago

The method is used by the function engine to store the latest value which was emitted by Func::Stream(), but this is redundant because the engine also stores that value in the engine's own table[vertex].

Moreover, most of the SetValue() implementations have a comment indicating that the function is a no-op, or is never called and is only implemented for consistency.

Expr::Value() is still useful because it is used during unification. For example, printf("%d", x) can tell the type checker that x must be an int, but only if the format argument's value is statically known. So it is still useful to have a method for asking for that value if it is known.

gelisam commented 1 year ago

I've figured it out: SetValue() is needed by StmtRes and friends, because it needs to use the latest value in order to run its side-effects, but it doesn't have access to the engine's table of cached values.

purpleidea commented 1 year ago

As a quick aside, I've been writing the new version of the engine to not use SetValue, and instead have obj.table be returned via a Table() method... Having said that, I'm still hitting some bugs that is blocking this from happening, and I'm not yet sure why. I guess this PR was prescient! ... If I can end up making it work that is!