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

Builtin for push() #108

Closed
jrha opened this issue Jun 6, 2016 · 17 comments
Closed

Builtin for push() #108

jrha opened this issue Jun 6, 2016 · 17 comments
Labels

Comments

@jrha
Copy link
Member

jrha commented Jun 6, 2016

As mentioned in #53 and #103 it would be good to have a push-like function as a compiler built-in.

If the left hand side of the assignment is a list, it should append each of the passed arguments to that list.
i.e.

'/foo' = list('spam');
'/foo' = push('beans', 'eggs');

Should result in:

"foo" : ["spam", "beans", "eggs"],

If the left hand side of the assignment is a dictionary, it should merge the passed arguments.
i.e.

'/foo' = dict('spam', true);
'/foo' = push('beans', false, 'eggs', false);

Should result in:

"foo" : {
    "spam" : true,
    "beans" : false,
    "eggs" : false,
},
@ned21
Copy link
Contributor

ned21 commented Jun 8, 2016

I am not keen on mixing push() for lists and dicts. Moving npush() to a compiler built-in would be nice I suppose but the separate names are helpful for understanding the type being manipulated.

@jrha
Copy link
Member Author

jrha commented Jun 8, 2016

npush or dpush or pushd ?

@DChambersSTFC DChambersSTFC self-assigned this Jun 9, 2016
@DChambersSTFC
Copy link

From what I can see, the Pan push() function works the same way as described here. Also, if the list being pushed to doesn't exist, an empty one will be created before being added to. If one of the arguments is a list itself, it will be appended as a list within the list being pushed to.

E.g

'/foo' = list('spam');
'/bar' = list('beans', 'eggs');
'/foo' = push(bar);

would result in

"foo" : ["spam", ["beans", "eggs"]],

For now, I will be trying to reproduce the behaviour of the Pan function into Java, without making any changes to behaviour.

@stdweird
Copy link
Member

stdweird commented Jun 9, 2016

can we also clarify the behviour in dml blocks, so we are certain quattor/template-library-core#104 is not "a feature"

@stdweird
Copy link
Member

stdweird commented Jun 9, 2016

@DChambersSTFC i'm not sure that mixing types in a list is a feature we want.
to proceed, we should first agree on the unittests (ie the pancode and the produced json profile)

@jrha
Copy link
Member Author

jrha commented Jun 10, 2016

@stdweird as discussed at the workshop, I'm pretty sure that quattor/template-library-core#104 isn't a feature, so it would be good to guard against it.

@DChambersSTFC
Copy link

Would it be appropriate to restrict all arguments to be the same type then?

@stdweird
Copy link
Member

yes, i think so.
@DChambersSTFC are you going to add these to pan (as in 'add code')? (i'm asking because we have a summerstudent starting in 2 weeks to have a look at pan and howto possibly add new builtins and refine some error messages)

@DChambersSTFC
Copy link

I'm working on adding them currently as I'm also new to pan and using this issue to learn my way around.

@jrha
Copy link
Member Author

jrha commented Jun 10, 2016

I would be good for @DChambersSTFC and your student to collaborate then!

@DChambersSTFC
Copy link

My main obstacle currently is referencing the object that called the function. Is anyone familiar on the use of the SetSelf function or knows of what I should be using?

@stdweird
Copy link
Member

stdweird commented Jun 10, 2016

@jrha @DChambersSTFC are you keeping notes how to add this (incl docs and unittests)? it's was basically the first thing we were going to ask our student: document how and were to add code and docs and tests for a new builtin. maybe they can meet during q4b?

@DChambersSTFC
Copy link

@loomis Do you know how I can reference the object calling the function?

@loomis
Copy link

loomis commented Jun 22, 2016

@DChambersSTFC The "Context" object passed into the functions contains the full context of the compilation, including the current state of the object being built and the reference to "self". See the "Context" interface for the full list of available methods.

Be very careful about mutating data structures that you access directly. They are often shared between object templates and direct mutation can cause concurrency issues or serialization of incorrect values.

@ned21
Copy link
Contributor

ned21 commented Apr 25, 2018

We decided at today's workshop in LAL that adding a built-in for push() would break all existing PAN code that uses the function from template-core. While cleaning up code might be possible, any change in behaviour between the current function and the built-in would lead to more subtle bugs so this is not worth the effort.

@jrha
Copy link
Member Author

jrha commented Apr 25, 2018

😞

@stdweird
Copy link
Member

@jrha there's still #87 to fix 😃

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

No branches or pull requests

5 participants