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

adds compiler support for object construction shorthand (full fields initializer) #21559

Closed
wants to merge 14 commits into from

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Mar 22, 2023

closes nim-lang/RFCs#418
closes nim-lang/RFCs#517

  • testing with defaults
  • object variants
  • private fields visible issues (still doesn't work with the last issue: private fields with generics)
  • mixing up with named fields
  • proper error messages
  • documentation
  • refactor replaceObjConstr and filterObjConstr into one proc returns enums
  • retry after failing the construction
type
  Vector = object
    a: int = 999
    b, c: int

block: # positional construction
  var x = Vector(1, 2, 3)
  echo x

block: # mixed with named parameters
  block:
    var x = Vector(a: 1, 2, 3)
    echo x

  block:
    var x = Vector(1, b: 2, 3)
    echo x

  block:
    var x = Vector(1, 2, c: 3)
    echo x

when false: # not allowed
  block:
     var x = Vector(b: 2, 3)
     echo x

type
  Color = enum
    Red, Blue, Yellow
  Factor = object
    id: int
    case flag: Color
    of Red:
      num: int
    of Blue, Yellow:
      done: bool
    name: string

block:
  var x = Factor(1, Red, 2, "1314")
  echo x

block:
  var x = Factor(1, Blue, true, "1314")
  echo x

block:
  var x = Factor(1, Yellow, false, "1314")
  echo x


type
  Ciao = object
    id: int
    case flag: bool = false
    of true:
      num: int
    of false:
      done: bool
    name: string

block:
  var x = Ciao(12, false, false, "123")
  echo x

block:
  var x = Ciao(12, flag: true, 1, "123")
  echo x

when false:  ## not allowed
  block:
    var x = Ciao(flag: true, 1, "123")
    echo x

Warning

Object constructions with a single unamed field will be interpreted as type conversions, which means

type
  Vector = object
    a: int

var x = Vector(1)

will get a type mismatch error.

@AngelEzquerra
Copy link
Contributor

It would be great to also support the shorthand proposed in this forum post: https://forum.nim-lang.org/t/10018#66156

let name = "Jim"
let group = "alpha group"
let p = Person( :name, :group )

This would be equivalent to:

let name = "Jim"
let group = "alpha group"
let p = Person(name: name, group: group )

The idea is to avoid the repetition of the parameter and variable names when they match, which is a pretty common pattern.

@ringabout
Copy link
Member Author

ringabout commented Mar 23, 2023

let p = Person( :name, :group )

It is not a valid Nim syntax, which means it needs a new syntax support or parser rewrites.

@Araq
Copy link
Member

Araq commented Mar 23, 2023

let p = Person( :name, :group ) is also quite ugly. Which is subjective, however colons in written natural language are neither postfix nor prefix but infix.

@AngelEzquerra
Copy link
Contributor

let p = Person( :name, :group ) is also quite ugly. Which is subjective, however colons in written natural language are neither postfix nor prefix but infix.

The original proposal in the thread was to use equals rather than colons (i.e. let p = Person( =name, =group )). Would that be better? It would be not very different to the similar pattern in the strformat "syntax". If not, is there any alternative syntax that you'd like?

I find that assigning a variable to a parameter of the same name is a relatively common pattern in my code, and every time I do it the repetition feels somewhat wrong. It would be nice to be able to avoid it if possible.

@Araq
Copy link
Member

Araq commented Mar 23, 2023

Would that be better?

No, that looks even worse. :-)

It would be not very different to the similar pattern in the strformat "syntax". If not, is there any alternative syntax that you'd like?

As this PR does it is fine. You can simply leave out the fieldname: and then the object field order is used, for example:

Vec3(1, 2, 3) or Vec3(1, y: 2, 3) etc. Named fields can be mixed with unnamed fields as you see fit, just like named arguments inside function calls.

@zedeus
Copy link
Contributor

zedeus commented Mar 25, 2023

The RFC mentioned proposes automatic mapping of variables to fields with the same name, like in JavaScript. This PR however is a different kind of shorthand that ignores field names and cares only about position (unless explicitly specified). These are two different, probably incompatible, solutions. Why was this chosen instead of the proposed solution in the RFC? That solution is closer to the current object construction syntax, e.g. User(name: name) becomes User(name). This also wouldn't break if the field order changes due to an optimization or refactoring which could silently break programs in cases where fields of the same type are switched around.

@ZoomRmc
Copy link
Contributor

ZoomRmc commented Mar 26, 2023

Sorry for writing everywhere at once, but doesn't this change require its own RFC?

I have some doubts about this syntax which I expressed in #418 this PR links to, even though this implementation is of a wider scope than the RFC.

PS: I deeply respect the hard work @ringabout has been doing for a while, so don't take it personally, the criticism has nothing to do with the quality of what's offered here.

@treeform
Copy link
Contributor

I think this is a great addition to the language. I know I will use it as soon as its available. I think it will really complement the default object values we have going out as well.

@Araq
Copy link
Member

Araq commented Mar 31, 2023

I guess this should be enabled via another --experimental switch but other than that, it would be nice to have for 2.0.

@Araq
Copy link
Member

Araq commented Mar 31, 2023

@ZoomRmc You're right that it needs an official RFC but please be assured that I've read every concern carefully.

@ringabout
Copy link
Member Author

ringabout commented Mar 31, 2023

Edited
done => nim-lang/RFCs#517

Sure thing, I will write the RFC.

@ringabout ringabout changed the title adds compiler support for object construction shorthand adds compiler support for object construction shorthand (full fields initializer) Mar 31, 2023
Copy link
Contributor

github-actions bot commented Apr 4, 2024

This pull request is stale because it has been open for 1 year with no activity. Contribute more commits on the pull request and rebase it on the latest devel, or it will be closed in 30 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Staled PR/issues; remove the label after fixing them label Apr 4, 2024
Copy link
Contributor

This pull request has been marked as stale and closed due to inactivity after 395 days.

@github-actions github-actions bot closed this May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Staled PR/issues; remove the label after fixing them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

compiler support for object construction shorthand (full fields initializer) Object init shorthand
6 participants