Nim icon indicating copy to clipboard operation
Nim copied to clipboard

add default field support for object in ARC/ORC

Open ringabout opened this issue 1 year ago • 5 comments

closes https://github.com/nim-lang/RFCs/issues/126 closes https://github.com/nim-lang/RFCs/issues/48 closes https://github.com/nim-lang/RFCs/issues/233 closes https://github.com/nim-lang/RFCs/issues/252 fixes #19763 fixes #16744 fixes #3608 ref https://github.com/nim-lang/RFCs/issues/437

inspired by https://github.com/nim-lang/Nim/pull/12378

default fields for object

  • [x] result
  • [x] object variant

the addition compared to https://github.com/nim-lang/Nim/pull/12378

  • [x] var
  • [x] omit type for default fields
  • [x] default(Type)
  • [x] new(ref)
  • [x] nested object definition
  • [x] array
  • [x] tuples
  • [x] an object of array etc. in the definition of the object
  • [x] threadvar
  • [x] noinit
  • [x] range
  • [x] fieldVisible issue
  • [x] distinct
  • [x] newFinalize
  • [x] seq(var, result, setLen)
  • [ ] docs
  • [x] tests

notes for me only

abstractInst vs {tyGenericInst, tyAlias, tySink}

ringabout avatar Aug 14 '22 05:08 ringabout

Why does the PR say "in ARC/ORC"? Shouldn't this be independent of the GC? Does this mean that the feature doesn't exist for other GCs or what's the deal?

konsumlamm avatar Aug 15 '22 08:08 konsumlamm

Why does the PR say "in ARC/ORC"? Shouldn't this be independent of the GC? Does this mean that the feature doesn't exist for other GCs or what's the deal?

I haven't figured out seq support for refc. I will focus on ARC/ORC first.

ringabout avatar Aug 15 '22 09:08 ringabout

To expand a bit on the issue with seqs @ringabout is referring to: refc and the other non-ARC/ORC memory management strategies use an RTTI-based seq implementation. The default field initialization as implemented here works by rewriting various statements and expressions during the semantic analysis phase.

In the seqs_v2 implementation used for ARC/ORC, initializing the elements to default values on sequence initialization or resize can be (and is) implemented by simply assigning all new elements to default(T) (which then gets rewritten during sem'), but the same isn't possible for the RTTI-based seq implementation, since the static type is erased there.

To @ringabout: a straightforward (though not necessarily the best) approach to make this work for the RTTI-based seqs would be to add a new init: proc field to TNimType, lift the default initialization logic for types into procedures, and adjust the genTypeInfoV1 procedure to also initialize the new TNimType.init field.

# the signature of the lifted init proc:
proc init(p: pointer)

proc setLengthSeqV2(s: PGenericSeq, typ: TNimType, newLen: int): PGenericSeq =
  ...
  for i in oldLen..<newLen:
    typ.base.init(dataPointer(result, elemAlign, elemSize, i))
  ...

# for `newSeq`, `cgen` adjustments would be required

Lifting the init procedures is maybe going to be a bit tricky - I believe introducing a new TTypeAttachedOp (e.g. attachedInit) and then performing the lifting during sempass2 could work.

To not create unused init procedures, only types appearing as the element type for seqs should get their initialization logic lifted.

zerbina avatar Aug 15 '22 17:08 zerbina

@zerbina Thanks for your incredible insights on this issue! I will give it a try afterwards.

ringabout avatar Aug 16 '22 13:08 ringabout

It's perfectly fine to combine default field support with ARC/ORC and only enable it for version 2.

Araq avatar Aug 24 '22 12:08 Araq

Please resolve conflicts.

And it needs a grammar update as @metagn suggested.

Araq avatar Sep 28 '22 11:09 Araq

Succeeded by https://github.com/nim-lang/Nim/pull/20480

ringabout avatar Oct 02 '22 09:10 ringabout