Stop evaluation of expressions with variables as operations

[vsbogd]

This is fast fix for #242 to unblock PLN work. This code doesn't use pragma! because at the moment pragma! settings are not reachable from the interpreter. More elegant fixes described in the issue could be implemented after migration to the minimal MeTTa.

[Necr0x0Der]

I'm ok with it if it works for @ngeiswei

[noskill]

i wonder how pln was blocked by matching first variable in an expression

[noskill]

also if the first variable can't be matched we can modify a program by adding dummy symbol:

(= (croaks Fritz) True)

(= (_ croaks Fritz) True)
!(let $res (_ $W Fritz) (if $res $W None))

So does this change actually changes anything?

[ngeiswei]

What do you think about adding a compile-time macro around that code? That way we can postpone the decision of whether to make it the default behavior and give ourselves more time to experiment with/without.

[vsbogd]

What do you think about adding a compile-time macro around that code? That way we can postpone the decision of whether to make it the default behavior and give ourselves more time to experiment with/without.

It is easy to do and I can do it. Do you suggest turn old or new behaviour by default?

Anyway I don't think it is a one way ticket and I consider this fix as a fast fix to unblock the PLN. As a proper fix I would like to add some method of control such matching in a MeTTa not event pragma, but custom eval for instance.

[ngeiswei]

Do you suggest turn old or new behaviour by default?

I don't mind either way, as long as the flag is mentioned in the README.md. Maybe old behavior could be the default to minimize disruption for now.

As a proper fix I would like to add some method of control such matching in a MeTTa not event pragma, but custom eval for instance.

👍

[vsbogd]

@ngeiswei , I added switch. @luketpeterson , could you please take a look to ensure REPL also has this feature turned on by default. I believe it is true, but I would ask you to double check.

[luketpeterson]

@ngeiswei , I added switch. @luketpeterson , could you please take a look to ensure REPL also has this feature turned on by default. I believe it is true, but I would ask you to double check.

Because the feature is a default in the top-level lib's Cargo.toml, all downstream dependencies (including repl) get the feature without any explicit change. Disabling a default feature is another story, however. Disagreement on that has been going for 7 years and shows no sign of getting any better. rust-lang/cargo#3126

I don't fully understand the rationale for providing this as a compile-time feature at all. It seems like we should find one behavior that works in all cases, and make whatever changes are needed to the interpreter so that is possible.

[Necr0x0Der]

I don't fully understand the rationale for providing this as a compile-time feature

It's a hotfix for the blocker right now. That's the rationale.

[luketpeterson]

I don't fully understand the rationale for providing this as a compile-time feature

It's a hotfix for the blocker right now. That's the rationale.

I am not questioning the change itself. You, Vitaly, & Nil have more clarity there.

My observation was that, by making the feature a library default, the set of downstream clients who will get the old behavior is likely to be the empty set. So I wasn't clear on the value of using the Cargo feature mechanism. But not a big deal if it's temporary.

[vsbogd]

Well, @ngeiswei requested this to be a compile time feature and he is the main user of the change, so I believe he will use it to unblock his work.

[vsbogd]

@ngeiswei , are you ok with current state of the PR?

[ngeiswei]

The set might not be empty cause I might be in it :-) I'd rather have an easy way to switch on/off in case I need to.

I'm happy with the state of the PR.

[vsbogd]

I'm happy with the state of the PR.

Ok, I am considering it as an approve :-)