Summary:	ASTERISK-11871: Can't add 0 in front of function result within if() block of AEL
Reporter:	Atis Lezdins (atis)	Labels:
Date Opened:	2008-04-17 08:23:23	Date Closed:	2008-04-21 16:07:44
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	PBX/pbx_ael
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	I'm trying to construct syntax that will allow to use function result directly in mathematical expression, and in case if function result is empty (NULL), interpret that as 0. This works fine from .conf file or realtime table: Set(calls_missed_queue=$[0${DB(agent_globals/${target_num}/calls_missed_queue_${call_from_queue})}+1]) Here's the line from AEL that doesn't work: if (${GROUP_COUNT(${target_num}@agent_queue_b)}=1+0${DB(skip_group/${target_num}/agent_queue_b)}); Seems that there's no way how to use this within if() check directly, it returns error at AEL compile time ("aelparse -d -n -w -q extensions.ael")
Comments:	By: Steve Murphy (murf) 2008-04-18 18:32:16 I fully understand what is going on now, and have made some moves to correct the situation. Firstly, I haven't played with 1.4 yet; but in trunk, these errors do not prevent aelparse from generating its output. Is this case with 1.4? If so, then you may not have so bitter a pill to swallow. This is the problem: AEL, after reading in the file and parsing it, runs a series of checks on the input dialplan. One of these passes the contents of if() (and other constructs) to the ast_expr2 stuff to see if it's syntactically OK. It's the ast_expr2 stuff that's complaining. Why? Because ${} and $[] constructs in the code are parsed as single tokens, and the expr2 grammar expects operators to be between tokens. The "0" and the ${...} tokens are next to each other and ast_expr2 reports a violation.  There is no problem with normal evaluation, however, because the algrorithm that evaluates these strings, always makes sure that the contents of any $[] or ${} contain no unevaluated $[] or ${} constructs, by recursively-bottom-up evaluating all these constructs out of the arugments. So, ast_expr2 in this regime never normally will see a ${} or $[] construct in its input. Just when used to syntax-check stuff in AEL. So, the first impulse was simply to drop the check. But, you know, such checks can be really valuable to developers in catching problems in exprs early. So, just for the sake of AEL, I added (in trunk), the binary operator "~~" which takes the two exprs before and behind it, and turns them into a single string with no space between the two input halves. I also included the code to not only parse it, but to evaluate it, so if it occurs in $[] exprs, it will do the right thing. Thus your "0${whatever}" would have to be rewritten "0~~${whatever}" to pass muster with AEL and ast_expr2. This has the added benefit of preparing the way for a future AEL3, where the grammar will be expanded to include $[] and ${} constructs directly, and we can do with the curly and square brackets around expressions entirely. Since spaces are not important to the grammar, this kind of formatting will need to be done a little bit more explicitly. So, for future AEL users: to get this kind of concatenation, you will have to insert the ~~ operator between such tokens. Now, for 1.4 and 1.6.0, I have the feeling that this kind of addition to ast_expr2 will not be appropriate, as it changes the grammar (true, it's backward compatible, etc, but it's still an enhancement). There are work-arounds, but, unfortunately, you have to toss in a few extra lines to do them, which I know you don't really like, but.... well, sorry. If you think I'm wrong, way off base, crazy/insane, let me know. Otherwise, I'll 'fix' this bug by introducing a new feature in trunk and close it out. By: Atis Lezdins (atis) 2008-04-21 05:16:05 Ok, true, this warning doesn't stop aelparse from generating output file. I noticed another thing with this - this syntax seems to be working in Set() - for example:  Set(test_if=$[0${DB(skip_group/${target_num}/agent_queue_b)}+1]);  if (${GROUP_COUNT(${target_num}@agent_queue_b)}=${test_if}) { gives no warnings. Ok, i don't completely understand all the internals of ast_expr2 and why is it so, but from my point of view i find it strange that syntax $[0${whatever}] works in Set() but doesn't work in if(). Maybe it's possible to add another check after ast_expr2, to see if it's constant concatenated? So, You propose do add ~~ for concatenation? That seems a bit weird and not intuitive. I would like something more traditional, for example "0$"{whatever} or "0${whatever}" By: Steve Murphy (murf) 2008-04-21 09:49:07 Why using this same expr in Set() gives no warnings: Because the pbx core, when it comes time to execute an application, evaluates the arguments before handing them to the application. It scans the arglist, and if it finds a $[] or ${} expr, it evaluates and replaces it in the string with its result. If the innards of the ${} or the $[] expr contains either ${} or $[] exprs, they are recursively evaluated out of the input until none are left. So, in this case, when an expr like $[ 0${x}  ] is evaluated, the pbx core sees first the outside $[] and would like to pass it to the $[] parser, which is the ast_expr2 stuff, but it scans the contents of the $[] and sees the ${x}. So, it will first evaluate the ${x} and replace it with, say for example, 101. So, ast_expr will always just see " 0101 ", which is just a single token, and that's that. This is done at run time, just before the app call is executed. In AEL, the Set() app call's arguments are simply passed thru without inspection. That's why you get no complaints. I had earlier put code in place to  evaluate app call args, but I had to remove it because of loading order, internal vs. external data availability, etc. if() expressions and For(x;y;z) exprs are still tested, tho. You see, the current app arg parser is pretty contextual and loose, and users use it to do two things simultaneously: string formatting and expression evaluation. This might seem like powerful features for users, but in the end, it creates a lot of subtle headaches, and makes parsers horribly complicated. There is also the SPRINTF function to do explicit string formatting; So, this is another alternative to building strings in the exact right format. And, the $[] parser has already been upgraded to handle calling functions inside exprs. By: Digium Subversion (svnbot) 2008-04-21 16:07:43 Repository: asterisk Revision: 114423 U   trunk/CHANGES U   trunk/doc/tex/ael.tex U   trunk/doc/tex/channelvariables.tex U   trunk/main/ast_expr2.c U   trunk/main/ast_expr2.fl U   trunk/main/ast_expr2.h U   trunk/main/ast_expr2.y U   trunk/main/ast_expr2f.c A   trunk/pbx/ael/ael-test/ael-ntest24/ A   trunk/pbx/ael/ael-test/ael-ntest24/extensions.ael U   trunk/pbx/ael/ael-test/ref.ael-ntest10 A   trunk/pbx/ael/ael-test/ref.ael-ntest24 U   trunk/pbx/ael/ael-test/ref.ael-test1 U   trunk/pbx/ael/ael-test/ref.ael-test18 U   trunk/pbx/ael/ael-test/ref.ael-test19 U   trunk/pbx/ael/ael-test/ref.ael-test3 U   trunk/pbx/ael/ael-test/ref.ael-test5 U   trunk/pbx/ael/ael-test/ref.ael-test8 U   trunk/pbx/ael/ael-test/ref.ael-vtest13 U   trunk/pbx/ael/ael-test/ref.ael-vtest17 ------------------------------------------------------------------------ r114423 | murf | 2008-04-21 16:07:41 -0500 (Mon, 21 Apr 2008) | 13 lines (closes issue ASTERISK-11871) Reported by: atis Tested by: murf This upgrade adds the ~~ (concatenation) string operator to expr2. While not needed in normal runtime pbx operation, it is needed when raw exprs are being syntax checked. This plays into future syntax- unification plans. By permission of atis, this addition in trunk and the reason of why things are as they are will suffice to close this bug. ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=114423