Solidity: Add parameter-less `push()` method to dynamic storage arrays (S)

[chriseth]

This method just adds a zero-initialized element to the end of the array and returns a storage reference to it.

• [ ] explain the differentce of x.push() = 7 and uint y = x.push(); y = 7; for x being a uint storage array in the documentation

[axic]

pushEmpty() for the sake of explicitness?

[chriseth]

We should discuss that, it might be that push() is easier to understand than pushEmpty()

[fulldecent]

Is there any demand for this variant?

Keyword additions should be considered critically, lest the dictionary run out of words for us to use.

[chriseth]

@fulldecent this does not add a keyword, it just adds a method to a predefined type. And yes, there is a demand for this, because in some cases, especially if you have an array of a complicated struct, it is hard to add a new element to an array if you cannot do .length++ anymore.

[kkirka]

Bare push() is ambiguous: it is unclear whether the value is default-initialized (e.g. with zeros) or not initialized at all (i.e. contains random stuff).

I think it is better to add default-initialization syntax for complex structs like in C++ and other languages. The syntax for initialization could be ComplexStruct cs = ComplexStruct(), and for pushing an empty element — .push(ComplexStruct()).

[chriseth]

@kkirka there is no possibility that anything contains random stuff in Solidity, everything is always zero-initialized.

While .push(ComplexStruct()) might be a possibility, it is much more work for the optimizer to improve the gas costs of such a construct (the struct is initialized in memory and then copied to storage).

I'm still torn between just adding push() and adding pushEmpty() or perhaps pushNew(). Are there any more opinitions?

[axic]

expand() increase(n) ?

[chriseth]

This function could either return the new length or a storage reference to the newly created element. The latter could be a little confusing for value types, though, since the two following snippets would have different behaviour:

x.pushEmpty() = 2; and uint b = x.pushEmpty(); b = 2;

[chriseth]

expand could be a good name, since we do not actually push anything.

[redsquirrel]

I think expand makes more sense than an empty push.

[fulldecent]

I vote for .push(). It is very clear what this does and it does not introduce new words into the language.

Not everybody speaks English. And those people need to learn EVERY word related to Solidity.

I had to deal with this problem working on ERC-721. "Title" was a great word for the concept we were creating. But this was very confusing to many people, especially ESL, so we chose "deed" (in addition to NFT).

[axic]

Some languages use grow().

[axic]

Can you please write down an example piece of code how this will be used?

I am not sure if this returns a storage reference or not? Otherwise will the user need to read .length and then retrieve a reference?

[chriseth]

Discussion resulted: inclining to just use push(). Need to come up with specific use-cases and decide whether it should return a reference to the new element, the new length or nothing.

[axic]

contract C {
  struct S { ... };
  S users[];

  // Returning a reference.
  // I think in this case allocate / pushEmpty / reserve could make sense (calling for other names :)
  function f() {
    S storage user = users.allocate();
  }

  // Just increasing the array.
  // I think in this case pushEmpty, expand, grow make sense
  function g() {
    users.push(); / users.pushEmpty(); / users.expand(); / users.grow()
    S storage user = users[users.length];
  }
}

[chriseth]

What about users.new()? or users.pushNew()? (which would then return the reference)

[chriseth]

Actually, I think returning the reference is really important, because juggling around with the length is rather combersome and might lead to errors.

[axic]

new sounds like a good option too.

Another one: would S storage = users.next() make sense? Probably sounds like an iterator.

Can we agree that we're going with the option of returning a reference and naming that appropriately?

[leonardoalt]

I agree with returning a reference.
new does sound good!

[chriseth]

new is actually not possible, since it is a keyword.

[OTTTO]

expand(n) or grow(n) makes sense, as there may be instances where a developer wants to add the default value and decides to use push() instead of push(0); in this case, the exact intentions may be confusing to someone reading the code, which is why a separate method name sounds appropriate.

[axic]

@ekpyron proposed pushZero

[chriseth]

Discussion: let's just start implementing this returning a reference and we can change the name later. Also we should check whether this is a breaking change for contracts that use using x for y.

[chriseth]

Example:

a.push().bla = 2;
StructName storage x = a.push();
x.bla = 2;
x.foo = 7;

would .pushNew() increase clarity?

[axic]

Looking at the examples I think pushEmpty sounds the most natural.

[chriseth]

empty makes sense for an array of arrays, but an array of uints? The problem is that we use the term "default value" or "zero value" a lot in the documentation but have not really defined or fixed it.

[ekpyron]

I think we should just use plain push() - I don't think that more explicitness has all that much real benefit here and it may even be confusing in the end - if you disagree I would probably prefer something like a.newElement() over pushNew (and pushEmpty indeed doesn't work in all cases).

[leonardoalt]

I agree with push()

[erak]

I'm also in favor of push(). Maybe we could think about returning (ref, new_length)?

[ekpyron]

@bit-shift Returning (ref, new_length) would make it difficult to use the returned reference without assigning everything to variables (think ìnitializeSomething(listOfSomethings.push())), so I'm in favor of just returning ref (having to use the length member of the array, if one needs the length seems fine with me).

[chriseth]

I think you usually don't care about the length.

[chriseth]

Decision by show of hands: let's call it push() and it returns a reference to the newly created array element.

[chriseth]

Implemented in https://github.com/ethereum/solidity/pull/7430