Table: mgetOrPut should be template

[inv2004]

Abstract

Convert table's mgetOrPut into template.

proc mgetOrPut*[A, B](t: var Table[A, B], key: A, val: B): var B

Because the val has to be evaluated every call, unrelated if key is in t or not

Motivation

There is template withValue, but it looks a bit overcomplicated

Description

The template I did in the example is not optimal - calls t[k] twice

Code Examples

import tables

var t = initTable[int, int]()

proc f(): int =
  echo "f"
  100

proc g(): int =
  echo "g"
  200

t.mgetOrPut(1, f()) += 3
t.mgetOrPut(1, f()) += 3

template mgetOrPutFix[A, B](t: var Table[A, B], key: A, val: B): var B =
  if key notin t:
    t[key] = val
  t[key]

t.mgetOrPutFix(2, g()) += 3
t.mgetOrPutFix(2, g()) += 3

echo t

f
f
g
{2: 206, 1: 106}

Backwards Compatibility

It is the main question why I created the RFC

[Araq]

It is the main question why I created the RFC

Somebody will have .borrow'ed mgetOrPut and this then stops working. That said, we should really change how .borrow is implemented so that it avoids these problems.

[mratsim]

Note: key in your example is evaluated 3 times and would be a bug if it had side effects like echo "launch missiles"

[inv2004]

@mratsim Yep, I wrote it in the "Description" - pretty sure my template is bad - it is just for the example

[c-blake]

Rather than change mgetOrPut, if you want a template, why not just add a new symbol? I called it editOrInit in adix/lptabz.

{ I think it's a good use case of what what I call "named do" as mentioned here. While one can do a macro to get it, most Nim metaprogrammers start with template and it's also a bit off that call(a=(x), b=(y)) is somehow more expressive than do notation in this named vs. ordinal parameter sense. This impacts naming since expected calls impact it, e.g. lookup edit.do: x init.do: y. }

[inv2004]

I decided to make mgetOrDefault for the moment - because most of initializations are ok with default values. It is not template

[inv2004]

@c-blake to move template discussion here unrelated to https://github.com/nim-lang/Nim/pull/22994

I think that convert mgetOrPut into template would be a bit better because:

1. it is free optimization for everyone who uses it
2. mgetOrPutLazy would could raise a lot of questions: why two functions, what is the reason to keep and use legacy function which is not optimized and etc. But I understand that it could takes years until the borrowed templates will be fixed
3. If remember correct pointer in withValue is not just part of internal implementation, but you have to deref it to use in one block. And it feels like one more duplicate like mgetOrPutLazy template

[c-blake]

Re: 1) Can it really be drop-in compatible / free optimization as a template since the new variant will have a block of code where the old variant had an expression evaluated pre-proc call? Maybe.. but the requirements / capability are perhaps different enough that a new name is warranted anyway for clarity rather than against it. { E.g., the new call can maybe even have two do: blocks. } Re: 3), just look at the runnableExamples with neither ptr nor [] in sight. In light of both, I think { Re: 2) } that it's worth it, and now { or almost 4 years ago for me :-) } just changing the name rather than waiting on compiler stuff to work. It's not a huge deal to me. I'm just trying to help with the thinking.