[Home]

Summary:ASTERISK-04347: [patch] Add conditional operator to the new expression parser
Reporter:Steve Murphy (murf)Labels:
Date Opened:2005-06-04 11:45:54Date Closed:2008-01-15 15:39:27.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) condexpr.patch
( 1) condexpr.patch2
( 2) condexpr.patch3
( 3) condexpr.patch4
Description:The idea was presented via the users' mailing list,
by Tony Mountifield, that the conditional operator ( c ? t : f ),
be added to the expression parser syntax. Thought this would
make an excellent enhancement.

****** ADDITIONAL INFORMATION ******

Only trouble is, the : collides with the : operator. So, I provide the
syntax:

c_expr ? true_expr :: false_expr

The double-colon is scanned as a single token, and no conflicts.

Here is an explanation of all the changes introduced by the patch:

a. In ast_expr2.fl, I added entries to return TOK_COND and TOK_COLONCOLON. I also removed ? as a character that built a general text token. As usual, if you want a non-alphanumeric char in a token, you can always wrap it double-quotes.

b. In ast_expr2.y, I added the op_cond() func, and its prototype. I added the TOK_COND and TOK_COLONCOLON tokens at the beginning of the precedence list, which gives them the lowest priority. I put TOK_COLON and TOK_EQTILDE at the same precedence level, as they are meant to serve almost the same function. I removed the UMINUS precedence placeholder, as it isn't necessary, and declared TOK_COMPL as right associative. I did this because a review of C++ grammar revealed that the unary operators are right associative (like the assignment ops). Best to follow the standards, even if it makes little difference. Amd. I added the new grammar rule for the conditional operator. I also removed the references to UMINUS from the unary operator rules, as the changes made these unnecessary.

c. In README.variables, I added a little explanation about precedence and associativity to the new unary operators, and the regex operators =~ and :. I added an entry to explain the conditional op. I changed the wording "conditional operator", to "conditional application" in the explanation about gotoif.


Comments:By: Kevin P. Fleming (kpfleming) 2005-06-05 11:06:09

Well, now we have some duplication with the IF() function. I do like the fact this is somewhat lighter weight, but I'm wondering if we really need both methods.

By: Steve Murphy (murf) 2005-06-09 22:39:34

Just in case my opinion counts for anything, I've looked at the code for
the $(IF func. Since it doesn't cause any evaluation to be done on it's parameters, and feeds its contents straight back out as a result, I've concluded that the evaluations are done before the IF func is performed. That means, that
if anything more than variable substitution is to be done, you'd have to use $[ ] exprs to do it.

So, yes, this addition to $[ ] is a little lighter weight, syntactically.

But, what the heck, it never hurts to have more than one way to do things. And I have to admit, if you must, simply must, have pure ? : syntax, mine is a loser, because you have to say ? :: instead.

So, my opinion is to have both methods. Let the users choose.

By: Steve Murphy (murf) 2005-06-14 09:40:04

Just added a new version of the patch, that adds a tweak to get rid of the warning message from the compiler, over the lack of a prototype for yylex.

By: Steve Murphy (murf) 2005-06-22 00:27:51

Ok, as advised in bug 0004556, I've uploaded another patch to add conditional exprs as explained before, to this bug.  The new version of the patch includes just one small set of lines about the conditional expr in the README.variables file, that was filtered in trying to gen the patch for the other patch.

And, as seemingly usual, along with a new patch, come a few more fixes. A few comments were removed. The ast_log replacement routine was upgraded to handle varargs, for standalone operation; see the check_expr program in utils.

By: Steve Murphy (murf) 2005-06-22 00:51:51

In keeping with my track record for not checking before uploading, I corrected a few mistakes in patch3, and submitted patch4.

A typo, that prevented STANDALONE operation, was fixed, and a long-standing problem (some compiler warnings) with the integer constants was fixed, by using "LL" with the constants instead of just "L". Some mixups in the replacement ast_log func were fixed.
I compiled, and tested the resulting standalone, testexpr2, and all is well.
Again, sorry for any hassle.

By: Kevin P. Fleming (kpfleming) 2005-06-23 22:51:25

Committed to CVS HEAD, thanks!

By: Digium Subversion (svnbot) 2008-01-15 15:39:27.000-0600

Repository: asterisk
Revision: 5997

U   trunk/ast_expr2.fl
U   trunk/ast_expr2.y
U   trunk/doc/README.variables

------------------------------------------------------------------------
r5997 | kpfleming | 2008-01-15 15:39:27 -0600 (Tue, 15 Jan 2008) | 2 lines

add conditional operator to expression parser, and various cleanups (bug ASTERISK-4347)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5997