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

VIP: Change Mapping Syntax #564

Closed
fubuloubu opened this Issue Dec 8, 2017 · 11 comments

Comments

6 participants
@fubuloubu
Member

fubuloubu commented Dec 8, 2017

Preamble

VIP: 564
Title: Change Mapping Syntax
Author: @fubuloubu @grosu
Type: Standard
Status: Draft
Created: 2017-12-08
Requires: #563

Abstract

Mapping syntax looks very similar to the array syntax, so we should change the syntax so it is not confusing

Motivation

See above

Specification

Change Mapping Syntax to
my_mapping: map(basetype1, type2)

Nested map Syntax to look as follows:
my_mapping: map(basetype1, map(basetype1, basetype2))

Backwards Compatibility

Not Backwards compatible

Copyright

Copyright and related rights waived via CC0


EDIT: Previous proposed syntax was my_mapping: basetype1 -> type2

@DavidKnott

This comment has been minimized.

Contributor

DavidKnott commented Dec 9, 2017

@fubuloubu Could you add an example of how nested mappings would look
(maybe my_mapping: basetype1 -> (basetype2 -> basetype3?)).

@fubuloubu

This comment has been minimized.

Member

fubuloubu commented Dec 9, 2017

my_mapping basetype1 -> basetype2 -> type3 I think that's pretty clear actually.

The harder part is how to represent the access e.g. my_mapping[acces1][access2] is how we do it now, but [ ] is the array syntax, I think we need to change it to something else.

There's also something like this in Haskell (I believe): my_mapping: (basetype1, basetype2) -> type3
Because this is how it works under the hood right? It hashes the two types together to obtain the location, right?

@fubuloubu fubuloubu changed the title from Change Mapping Syntax to VIP: Change Mapping Syntax Dec 18, 2017

@fubuloubu

This comment has been minimized.

Member

fubuloubu commented Dec 18, 2017

Meeting Notes:

Syntax would be more explicit like this

my_mapping1: map(basetype1 -> type2)
my_mapping2: map(basetype1 -> basetype2 -> type3)
my_mapping3: map(bytes <= 20 -> type2)
@yzhang90

This comment has been minimized.

yzhang90 commented Mar 22, 2018

I like the syntax proposed by @jacqueswww :
a: public(map({int128: {bytes32: int128}}) in case of a nested map.

@fubuloubu

This comment has been minimized.

Member

fubuloubu commented Mar 22, 2018

I sorta like it, although the syntax of structures using braces is also fairly close. Only difference is that only types can be used. Seems confusing.

@SamuelTroper

This comment has been minimized.

Contributor

SamuelTroper commented Mar 29, 2018

I'm actually a fan of the syntax now. It's one of the things that I genuinely think is an improvement over most major languages. It's short, simple, and intuitive once you do it once or twice.

It could be interesting to use something else to enclose it perhaps. Maybe a declaration along the lines of my_mapping: basetype1<basetype2> which would make it instantly distinguishable from a list, but would keep the syntax similar. To access, you could do something like my_mapping<access> which parallels the declaration as well. It could also be a cool distinguishing feature of the language, and if < and > are used it could become know as Vyper Fangs.

@fubuloubu

This comment has been minimized.

Member

fubuloubu commented Mar 29, 2018

@jacqueswww I think we need to discuss this one on our next call. Lots of options here.

@jacqueswww jacqueswww added this to To do in Fix the Beta via automation Aug 11, 2018

@jacqueswww jacqueswww added Approved and removed post beta labels Nov 14, 2018

@fubuloubu

This comment has been minimized.

Member

fubuloubu commented Nov 14, 2018

This proposal was updated per discussion in #1076

@jakerockland

This comment has been minimized.

Contributor

jakerockland commented Nov 14, 2018

I'm a fan of the new proposed my_mapping: map(basetype1, type2)

@jacqueswww jacqueswww moved this from To do to In progress in Fix the Beta Nov 15, 2018

@jacqueswww

This comment has been minimized.

Contributor

jacqueswww commented Nov 15, 2018

Starting on this.

@fubuloubu

This comment has been minimized.

Member

fubuloubu commented Nov 15, 2018

@jacqueswww maybe we should deprecate the old syntax with a warning to move to the new one so people know to switch?

Fix the Beta automation moved this from In progress to Done Nov 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment