weld icon indicating copy to clipboard operation
weld copied to clipboard

Struct builders

Open segeljakt opened this issue 5 years ago • 4 comments

The docs say:

Any struct whose fields are builders can also be used as a builder. This is used to build multiple results at the same time.

But, when I compile:

|source: vec[i32]|
    let final = for(source, {appender[i32], appender[i32]}, |sink, index, element|
        merge(sink, {element, element})
    );
    {result(final.$0), result(final.$1)}

I get:

[info] 15:49:09.591: Started weld_module_compile
[info] 15:49:09.594: Started parsing program
[info] 15:49:09.608: Done parsing program
[info] 15:49:09.608: Started compiling program
[debug] 15:49:09.611: After macro substitution:
|source:vec[i32]|
  (let final:?=(for(
    source:?,
    {appender[i32],appender[i32]},
    |sink:?,index:?,element:?|
      merge(sink:?,{element:?,element:?})
  ));{result(
    final.$0
  ),result(
    final.$1
  )})

[info] 15:49:09.612: Done compiling program
REPL: Compile error: Internal error: Merge called on non-builder
>>>

My expectation was that you could merge structs which only contain builders, and also call result on structs which only contain builders.

segeljakt avatar Jul 19 '19 13:07 segeljakt

Try changing your merge to be {merge(sink.$0, element), merge(sink.$1, element)} (you need to explicitly specify the merge for each builder in the struct).

On Fri, Jul 19, 2019 at 6:50 AM segeljakt [email protected] wrote:

The docs say:

Any struct whose fields are builders can also be used as a builder. This is used to build multiple results at the same time.

But, when I compile:

|source: vec[i32]| let final = for(source, {appender[i32], appender[i32]}, |sink, index, element| merge(sink, {element, element}) ); {result(final.$0), result(final.$1)}

I get:

[info] 15:49:09.591: Started weld_module_compile [info] 15:49:09.594: Started parsing program [info] 15:49:09.608: Done parsing program [info] 15:49:09.608: Started compiling program [debug] 15:49:09.611: After macro substitution: |source:vec[i32]| (let final:?=(for( source:?, {appender[i32],appender[i32]}, |sink:?,index:?,element:?| merge(sink:?,{element:?,element:?}) ));{result( final.$0 ),result( final.$1 )})

[info] 15:49:09.612: Done compiling program REPL: Compile error: Internal error: Merge called on non-builder

My expectation was that you could merge structs which only contain builders, and also call result on structs which only contain builders.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/weld-project/weld/issues/464?email_source=notifications&email_token=AAKMEY5UTCAG6ZFX3S6B5U3QAHBDJA5CNFSM4IFGFODKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HAIYPGA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKMEY7AVMZRA2UALOSFILDQAHBDJANCNFSM4IFGFODA .

-- Shoumik

sppalkia avatar Jul 19 '19 14:07 sppalkia

Ok, thanks, makes sense :)

I have another question regarding builders. What happens internally in Weld when you swap the field positions of two builders? E.g:

|source: vec[i32]|
    let final = for(source, {appender[i32], appender[i32]}, |sink, index, element|
        {merge(sink.$1, 1), merge(sink.$0, 0)}
    );
    {result(final.$0), result(final.$1)}

segeljakt avatar Jul 19 '19 15:07 segeljakt

That's fine as long as you maintain their linearity property (i.e., each builder should be consumed exactly once per control path) and the types of the swapped builders are the same if you're inside a for loop. We don't have a compile time check for this at the moment unfortunately (it's in the works though!)

On Fri, Jul 19, 2019 at 8:09 AM segeljakt [email protected] wrote:

Ok, thanks, makes sense :)

I have another question regarding builders. What happens internally in Weld when you swap the field positions of two builders? E.g:

|source: vec[i32]| let final = for(source, {appender[i32], appender[i32]}, |sink, index, element| {merge(sink.$1, 1), merge(sink.$0, 0)} ); {result(final.$0), result(final.$1)}

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/weld-project/weld/issues/464?email_source=notifications&email_token=AAKMEY776IQO2UN2HVRVQHTQAHKLVA5CNFSM4IFGFODKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2L5DZQ#issuecomment-513266150, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKMEY6NPDYUVNSAC2266NTQAHKLVANCNFSM4IFGFODA .

-- Shoumik

sppalkia avatar Jul 19 '19 15:07 sppalkia

Sorry, realized you asked what happens internally: you can think of the builder as a C struct, so swapping them within a struct will effectively swap the struct values.

On Fri, Jul 19, 2019 at 8:46 AM Shoumik Palkar [email protected] wrote:

That's fine as long as you maintain their linearity property (i.e., each builder should be consumed exactly once per control path) and the types of the swapped builders are the same if you're inside a for loop. We don't have a compile time check for this at the moment unfortunately (it's in the works though!)

On Fri, Jul 19, 2019 at 8:09 AM segeljakt [email protected] wrote:

Ok, thanks, makes sense :)

I have another question regarding builders. What happens internally in Weld when you swap the field positions of two builders? E.g:

|source: vec[i32]| let final = for(source, {appender[i32], appender[i32]}, |sink, index, element| {merge(sink.$1, 1), merge(sink.$0, 0)} ); {result(final.$0), result(final.$1)}

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/weld-project/weld/issues/464?email_source=notifications&email_token=AAKMEY776IQO2UN2HVRVQHTQAHKLVA5CNFSM4IFGFODKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2L5DZQ#issuecomment-513266150, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKMEY6NPDYUVNSAC2266NTQAHKLVANCNFSM4IFGFODA .

-- Shoumik

-- Shoumik

sppalkia avatar Jul 19 '19 15:07 sppalkia