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

Update behavior of augmented null coalescing operator #3614

Conversation

d-torrance
Copy link
Member

x ??= y now behaves like x ?? (x = y) rather than x = x ?? y.

These are equivalent when x is null.

When x is non-null, we avoid an unnecessary re-assignment step, which also allows for the possibility of using this operator with immutable hash tables without raising errors when keys already exist.

Closes: #3612

x ??= y now behaves like x ?? (x = y) rather than x = x ?? y

These are equivalent when x is null.

When x is non-null, we avoid an unnecessary re-assignment step, which
also allows for the possibility of using this operator with immutable
hash tables without raising errors when keys already exist.

Closes: Macaulay2#3612
@d-torrance d-torrance requested a review from mahrud December 17, 2024 12:56
then (
e := tryEval(x.lhs);
if tryEvalSuccess then lexpr = e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused. Where is the actual assignment to lexpr happening now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need it anymore when the left-hand side is non-null since we return right away. And when it's null, we've already assigned lexpr as nullE a couple lines earlier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still confused. Where does the x = y in x ?? (x = y) is happening? I thought here lexpr was x.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assignment happens in one of several places, depending on what the code looks like.

lexpr is the evaluated left-hand side. But we still have the original unevaluated Code object, which we need to figure out what type of assignment we'll end up doing (global, local, assigning an element of a list/hash table, etc.). Here's the basic gist:

  • Go ahead and evaluate the left hand side (lexpr) and stuff it in an evaluatedCode object (left). We also check for errors here and whether someone has installed an augmented assignment method for this operator for the class of lexpr. If so, call that and return the result.
  • Now go back and look at the original unevaluated Code object to figure out how to assign things. But instead of using this Code object, we swap it out with left to avoid re-evaluating the left-hand side.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that makes sense.

@mahrud
Copy link
Member

mahrud commented Dec 17, 2024

Unrelated to this PR, but I just noticed this:

i1 : (a,b) ??= (1,2)
-*dummy position*- error: augmented assignment not implemented for this code

@d-torrance d-torrance merged commit 431907b into Macaulay2:development Dec 17, 2024
5 checks passed
@d-torrance
Copy link
Member Author

d-torrance commented Dec 17, 2024

Ok yeah -- at the very least we should fix the -*dummy position*- in the message. But I suppose it would be cool if this worked like a = a + 1; b = b + 2.

@mahrud
Copy link
Member

mahrud commented Dec 17, 2024

Yeah, I don't necessarily need that to work (at least it's not a priority), but the dummy position surprised me.

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

Successfully merging this pull request may close these issues.

2 participants