Enrich document with using getter function over state struct instance #15526

haoyang9804 · 2024-10-18T21:23:10Z

Inspired by this issue: #15525. I guess it's better to add a document for this special return type since it's truly confusing.

github-actions · 2024-10-18T21:23:25Z

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

docs/contracts/visibility-and-getters.rst

cameel

Just a note on general structure here: the section on getters is already a bit too verbose IMO, with a lot of space and examples given to what are essentially obscure corner cases. I think we shouldn't make it even longer. We should consolidate the examples and make text more coherent. The focus should be kept primarily on explaining what getters are, the other things are just notes.

haoyang9804 · 2024-10-21T14:02:55Z

Just a note on general structure here: the section on getters is already a bit too verbose IMO, with a lot of space and examples given to what are essentially obscure corner cases. I think we shouldn't make it even longer. We should consolidate the examples and make text more coherent. The focus should be kept primarily on explaining what getters are, the other things are just notes.

How about merging struct example and mapping example? They are all corner cases that can lead to confusion.

cameel · 2024-10-21T14:29:20Z

Yes, they should be consolidated IMO. We could have just one example showing them all. Or even just a general description of how the ABI for a getter looks like. Which is relevant e.g. when you want to override a function with a getter and should be mentioned too.

haoyang9804 · 2024-10-21T14:47:48Z

Yes, they should be consolidated IMO. We could have just one example showing them all. Or even just a general description of how the ABI for a getter looks like. Which is relevant e.g. when you want to override a function with a getter and should be mentioned too.

Good suggestion. But I can only help with the consolidation part. I'm not very familiar with ABI.

haoyang9804 · 2024-10-21T16:47:50Z

I think the Complex example is useful for benefit users' understanding in getter function. Maybe we can remove this example and only leave examples of array and struct the mitigate confusion.

haoyang9804 · 2024-10-21T17:27:28Z

I think the Complex example is useful for benefit users' understanding in getter function. Maybe we can remove this example and only leave examples of array and struct the mitigate confusion.

I have merged the struct example and the array example. As for the Complex one that combine mapping and struct, since it implies no corner case, I removed it.
As for the ABI part, I think I can submit another pr after gaining some background.

haoyang9804 · 2024-10-25T08:05:05Z

@cameel Hi Kamil, any suggestions on the new commit?

matheusaaguiar

I don't agree with the removal of the "complex example". It illustrates a case which we have to include, and which without mentioning it, the proposed added text of this PR would be incomplete.

The getter function of a struct returns a tuple whose elements are each individual struct member. But, as the complex example illustrates, in some cases they are omitted.
A simpler example would be the following:

contract C {
    struct S {
        int[2] a;
        uint256 x;
    }
S s;
}

In this case, a is omitted and the getter return type would be (uint256).

docs/contracts/visibility-and-getters.rst

haoyang9804 · 2024-10-25T22:00:41Z

Thx for your review @matheusaaguiar, I have refined the doc, please take a look

matheusaaguiar · 2024-10-29T15:33:38Z

docs/contracts/visibility-and-getters.rst

-    contract Complex {
-        struct Data {
+        // Struct Definition with multiple members
+        struct ComplexStruct {
            uint a;
-            bytes3 b;
-            mapping(uint => uint) map;
-            uint[3] c;
-            uint[] d;
-            bytes e;
+            bool b;
+            int[2] arr;
        }
-        mapping(uint => mapping(bool => Data[])) public data;
-    }

-It generates a function of the following form. The mapping and arrays (with the
-exception of byte arrays) in the struct are omitted because there is no good way
-to select individual struct members or provide a key for the mapping:


I think this should stay.

I have merged it with the ComplexStruct

matheusaaguiar · 2024-10-29T15:35:53Z

docs/contracts/visibility-and-getters.rst

+rather than the struct as a single object in memory.
+The members returned appear in the order they are declared in the struct, provided they
+are not omitted. Array-type members within the struct are excluded from the returned tuple.
+Additionally, if the struct consists solely of array-type members, no getter function will be generated


Not only arrays but also mappings don't have a getter generated (in this case) because as explained in the "Complex Example":
there is no good way to select individual struct members or provide a key for the mapping

Also, I guess that in the above quote, it was meant to be array members instead of struct members originally.

have changed it

Also, I guess that in the above quote, it was meant to be array members instead of struct members originally.

Can someone else check and see if they interpret it the same way I did? @r0qs @nikola-matic @clonker

haoyang9804 · 2024-11-04T08:39:22Z

Hi @matheusaaguiar , is it OK to merge?

matheusaaguiar

Hi @haoyang9804, sorry for the late response. The team was busy during this month attending devcon.
I think this is a good state for this section now.
@mehtavishwa30 please could you give a final look and also confirm my suggestion in #15526 (comment) ?

matheusaaguiar · 2024-11-29T14:55:18Z

docs/contracts/visibility-and-getters.rst


-The next example is more complex:
+When you declare a public state variable of a struct type, the generated getter function
+returns specific member of the struct as separate elements within a tuple,


Suggested change

returns specific member of the struct as separate elements within a tuple,

returns each member of the struct as separate elements within a tuple,

matheusaaguiar · 2024-11-29T14:57:14Z

docs/contracts/visibility-and-getters.rst

-        b = data[arg1][arg2][arg3].b;
-        e = data[arg1][arg2][arg3].e;
-    }
+        // Getter function generated by the compiler


I think it would be nice to keep the explanation from before (with the correction i suggested in #15526 (comment)):

The mapping and arrays (with the exception of byte arrays) in the struct are omitted because there is no good way to select individual array members or provide a key for the mapping

have changed based on my understanding. plz check

github-actions bot added the external contribution ⭐ label Oct 18, 2024

cameel added the documentation 📖 label Oct 18, 2024

cameel requested review from nikola-matic, matheusaaguiar and r0qs and removed request for matheusaaguiar October 18, 2024 22:32

nikola-matic reviewed Oct 21, 2024

View reviewed changes

r0qs reviewed Oct 21, 2024

View reviewed changes

docs/contracts/visibility-and-getters.rst Outdated Show resolved Hide resolved

cameel reviewed Oct 21, 2024

View reviewed changes

haoyang9804 requested review from cameel, r0qs and nikola-matic October 25, 2024 08:16

matheusaaguiar requested changes Oct 25, 2024

View reviewed changes

docs/contracts/visibility-and-getters.rst Outdated Show resolved Hide resolved

docs/contracts/visibility-and-getters.rst Outdated Show resolved Hide resolved

docs/contracts/visibility-and-getters.rst Outdated Show resolved Hide resolved

haoyang9804 requested a review from matheusaaguiar October 25, 2024 22:01

matheusaaguiar reviewed Oct 29, 2024

View reviewed changes

haoyang9804 requested a review from matheusaaguiar October 30, 2024 11:20

matheusaaguiar previously approved these changes Nov 29, 2024

View reviewed changes

haoyang9804 added 3 commits December 2, 2024 11:57

enrich document with using getter function over state struct instance

f4cb57f

fix errors in the doc

81b8a67

remove Trailing whitespace

464cb00

haoyang9804 added 9 commits December 2, 2024 11:57

Refine doc based on suggestions

0251f9e

refine the doc

f02c816

merge array example and struct example. Remove mapping.

f043514

Document a special workaround for calldata arrays and struct instances

c2ce59b

reset the incorrect commit

d6010a2

refine the doc

e1dd479

Fix errors in the code example

abe0934

refine the doc

5d4fe82

update

6173e00

haoyang9804 dismissed matheusaaguiar’s stale review via 6173e00 December 2, 2024 11:59

haoyang9804 force-pushed the develop branch from 7a0e70d to 6173e00 Compare December 2, 2024 11:59

haoyang9804 requested a review from matheusaaguiar December 2, 2024 12:00

matheusaaguiar approved these changes Dec 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enrich document with using getter function over state struct instance #15526

Enrich document with using getter function over state struct instance #15526

haoyang9804 commented Oct 18, 2024 •

edited

Loading

github-actions bot commented Oct 18, 2024

cameel left a comment

haoyang9804 commented Oct 21, 2024

cameel commented Oct 21, 2024

haoyang9804 commented Oct 21, 2024

haoyang9804 commented Oct 21, 2024

haoyang9804 commented Oct 21, 2024

haoyang9804 commented Oct 25, 2024

matheusaaguiar left a comment •

edited

Loading

haoyang9804 commented Oct 25, 2024

matheusaaguiar Oct 29, 2024

haoyang9804 Oct 30, 2024

matheusaaguiar Oct 29, 2024 •

edited

Loading

haoyang9804 Oct 30, 2024

matheusaaguiar Oct 30, 2024

haoyang9804 commented Nov 4, 2024

matheusaaguiar left a comment

matheusaaguiar Nov 29, 2024

haoyang9804 Dec 2, 2024

matheusaaguiar Nov 29, 2024

haoyang9804 Dec 2, 2024

	returns specific member of the struct as separate elements within a tuple,
	returns each member of the struct as separate elements within a tuple,

Enrich document with using getter function over state struct instance #15526

Are you sure you want to change the base?

Enrich document with using getter function over state struct instance #15526

Conversation

haoyang9804 commented Oct 18, 2024 • edited Loading

github-actions bot commented Oct 18, 2024

cameel left a comment

Choose a reason for hiding this comment

haoyang9804 commented Oct 21, 2024

cameel commented Oct 21, 2024

haoyang9804 commented Oct 21, 2024

haoyang9804 commented Oct 21, 2024

haoyang9804 commented Oct 21, 2024

haoyang9804 commented Oct 25, 2024

matheusaaguiar left a comment • edited Loading

Choose a reason for hiding this comment

haoyang9804 commented Oct 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matheusaaguiar Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haoyang9804 commented Nov 4, 2024

matheusaaguiar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haoyang9804 commented Oct 18, 2024 •

edited

Loading

matheusaaguiar left a comment •

edited

Loading

matheusaaguiar Oct 29, 2024 •

edited

Loading