On 2013-05-22 08:18, Iyer, Balaji V wrote:
The overall function names are same, but the components inside it function
differs greatly from C and C++. For example, in C++ I can't use
build_modify_expr, but build_x_modify_expr. Also, I need to handle
overloaded function types, and that requires further checking. Similarly,
the trees are different from C and C++, and so I have to do more checks and
handle them differently in C++. In my mind, this seem to be the most
straight-forward and legible way to do it.
Really, that's surprising:
/* For use from the C common bits. */
tree
build_modify_expr (location_t /*location*/,
tree lhs, tree /*lhs_origtype*/,
enum tree_code modifycode,
location_t /*rhs_location*/, tree rhs,
tree /*rhs_origtype*/)
{
return cp_build_modify_expr (lhs, modifycode, rhs, tf_warning_or_error);
}
I can definitely see how function overloading could potentially get in the way.
Although by delaying expansion of ARRAY_NOTATION_REF, given that the TREE_TYPE
of the ARRAY_NOTATION_REF is not some complex artificial slice type but the
element type of the array, I wonder how much of the overloaded function
selection would just happen automatically?
Which reminds me: What is ARRAY_NOTATION_TYPE for? Isn't it always the same as
the TREE_TYPE of the ARRAY_NOTATION_REF?
Before I started my implementation, I did briefly consider using
c_finish_loop, but adding multiple expressions got a bit complicated. For
example, if I have 2 increment/condition expressions (my initial
implementation had it but later on I eliminated that need), I had to create
a statement list and then add them using append_to_statement_list() and then
pass that into c_finish_loop. Also, if I am not mistaken, I still had to
create some of the individual labels when using c_finish_loop(). Overall, I
didn't find much code-size decrease, and I found this a bit more straight
forward for me. I apologize if I am mistaken.
Well, append_to_statement_list etc was certainly not needed. Mirroring exactly
how you'd write it in C, with a comma operator also works. That's a
COMPOUND_EXPR, fyi.
There are continue/break label arguments to c_finish_loop, but they may be (and
often are) null. They will only be non-null if the relevant statements
actually exist inside the loop. C.f. c_finish_bc_stmt and c_break_label.
You're right about it not saving *that* much code, but the point is more about
if we make improvements to normal C loop representation -- such as to support
#pragma simd -- then we want to be able to take advantage of that here.
Are these changes a blocking issue for acceptance into trunk? Or, would it
be ok to accept it as-is and then I make these changes at a later date?
I'm leaning to honoring the advice you were given last year and accepting the
patch more or less in its current form.
r~
