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

Better syntax for declaring nested mappings. #8499

Closed
Amxx opened this issue Mar 17, 2020 · 7 comments
Closed

Better syntax for declaring nested mappings. #8499

Amxx opened this issue Mar 17, 2020 · 7 comments
Labels
language design :rage4: Any changes to the language, e.g. new features low impact Changes are not very noticeable or potential benefits are limited. medium effort Default level of effort needs design The proposal is too vague to be implemented right away waiting for more input Issues waiting for more input by the reporter
Projects

Comments

@Amxx
Copy link

Amxx commented Mar 17, 2020

At some point today I found myself declaring the following mapping:

mapping(bytes32 => mapping(bytes32 => mapping(bytes32 => mapping(uint256 => MyEntry)))) internal m_lookuptable;

I get this is a bit of an extreme case, but I might not be the only one nesting mappings. So I wondered, why not replace it with:

mapping(bytes32 => bytes32 => bytes32 => uint256 => MyEntry) internal m_lookuptable;

It should be pretty easy for the parser to adapt to this syntax, no backend modification should be necessary, and the result would be much more readable.
Also, it doesn't break anything, so backward compatibility would be unaffected.

@chriseth chriseth added language design :rage4: Any changes to the language, e.g. new features to discuss (design) labels Mar 19, 2020
@chriseth
Copy link
Contributor

Thanks for your suggestion, @amaxx! I'm wondering whether you really need a data structure that support "partial evaluation" or if what you really need is tuples as keys for mappings.

So it would rather be

mapping(bytes32, bytes32, bytes32, uint256 => MyEntry) internal m_lookuptable;

and you would use m_lookuptable["1", "2", "3", 1] instead of m_lookuptable["1"]["2"]["3"][1]

It would also have the benefit that you only need to compute one hash instead of 4.

@chriseth chriseth added the waiting for more input Issues waiting for more input by the reporter label Apr 20, 2020
@axic
Copy link
Member

axic commented Apr 20, 2020

Perhaps using the tuple syntax we have is more clear:

mapping((bytes32, bytes32, bytes32, uint256) => MyEntry) internal m_lookuptable;

@elenadimitrova elenadimitrova added this to New issues in Solidity via automation May 14, 2020
@elenadimitrova elenadimitrova moved this from New issues to Design Backlog in Solidity May 14, 2020
@axic
Copy link
Member

axic commented Mar 25, 2021

Btw @Amxx, it occurred to me that you can (almost) already do this albeit at a memory cost: can use a single bytes32 key and hash your keys (keccak256(abi.encode(a, b, c,...))). With bytes.concat (#10903) this should be clarified to keccak256(bytes.concat(a, b, c, ...).

So basically the tuple syntax is a memory-efficient and more easy to ready alternative to the above.

We should keep in mind that potentially the codegen should be able to avoid the memory overhead, if it is clever enough.

@cameel cameel added medium effort Default level of effort low impact Changes are not very noticeable or potential benefits are limited. needs design The proposal is too vague to be implemented right away labels Sep 26, 2022
@cameel
Copy link
Member

cameel commented Feb 6, 2023

We discussed it briefly today and it does not look like it's compatible with the direction we want the language to go. It's a nice bit of syntactic sugar but conflicts with the fact that we'd like mappings to eventually stop being special and become a user-defined generic type provided by the standard library, implementable using other language primitives.

We're also planning to introduce proper, nameable tuple types at some point. This should be a good enough alternative to this feature and would work the way @axic has shown above: #8499 (comment).

@cameel cameel closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2023
Solidity automation moved this from Design Backlog to Done Feb 6, 2023
@Amxx
Copy link
Author

Amxx commented Feb 6, 2023

We're also planning to introduce proper, nameable tuple types at some point. This should be a good enough alternative to this feature and would work the way @axic has shown above: #8499 (comment).

I would consider this a solution to this issue! Any timeframe on this ?

@cameel
Copy link
Member

cameel commented Feb 6, 2023

Not sure actually. It's not directly on the roadmap but I think it might be a prerequisite of some other roadmap task. @ekpyron?

@ekpyron
Copy link
Member

ekpyron commented Feb 6, 2023

We haven't really discussed the timeframe of those yet - it's one of the issues that makes our type system more regular and would prepare it for an easier transition to a type system including full generics, like tying data locations to types instead of variables. We could target them in the vicinity of #13365 (which is staged for Q2), but yeah, not entirely clear yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language design :rage4: Any changes to the language, e.g. new features low impact Changes are not very noticeable or potential benefits are limited. medium effort Default level of effort needs design The proposal is too vague to be implemented right away waiting for more input Issues waiting for more input by the reporter
Projects
No open projects
Solidity
  
Done
Development

No branches or pull requests

6 participants