On 11/19/2014 05:24 PM, David Malcolm wrote:
On Wed, 2014-11-19 at 22:36 +0100, Richard Biener wrote:
On November 19, 2014 10:09:56 PM CET, Andrew MacLeod <amacl...@redhat.com>
wrote:
On 11/19/2014 03:43 PM, Richard Biener wrote:
On November 19, 2014 8:26:23 PM CET, Andrew MacLeod
<amacl...@redhat.com> wrote:
On 11/19/2014 01:12 PM, David Malcolm wrote:
(A) could become:
greturn *stmt = gsi->as_a_greturn ();
(B) could become:
stmt = gsi->dyn_cast <gcall *> ();
if (!stmt)
or:
stmt = gsi->dyn_cast_gcall ();
if (!stmt)
or maybe:
stmt = gsi->is_a_gcall ();
if (!stmt)
An earlier version of my patches had casting methods within the
gimple_statement_base class, which were rejected; the above
proposals
would instead put them within gimple_stmt_iterator.
I would like all gsi routines to be consistent, not a mix of
functions
and methods.
so something like
stmt = gsi_as_call (gsi);
stmt = gsi_dyn_call (gsi);
or we need to change gsi_stmt() and friends into methods and access
them
as gsi->stmt ().. which is possibly better, but that much more
intrusive again (another 2000+ locations).
If we switched to methods everywhere for gsi, I'd prefer something
like
gsi->as_a_call ()
gsi->is_a_call ()
gsi->dyn_cast_call ()
I think its more readable... and it removes a dependency on the
implementation.. so if we ever decide to change the name of 'gcall'
down
the road to using a namespace, and make it gimple::call or whatever,
we
wont have to change every single gsi-> location which has a
templated
use of the type.
I'm also think this sort of thing could probably wait until next
stage
1..
my 2 cents...
Why not as_a <gassign *> (*gsi)? It would
Add operator* to gsi of course.
Richard.
I could live with that form too.
we often have an instance of gimple_stmt_iterator rather than a pointer
to it, so wouldn't "operator gimple *()" to implicitly call gsi_stmt()
when needed work better? (or "operator gimple ()" before the next
change) ..
Not sure. The * matches how iterators work in STL...
Note that for the cases where we pass a pointer to an iterator we can change
those to use references to avoid writing **gsi.
Richard.
Andrew
I had a go at adding an operator * to gimple_stmt_iterator, using it
everywhere that we do an as_a<> or dyn_cast<> on the result of a
gsi_stmt, to abbreviate the gsi_stmt call down to one character.
Patch attached; only lightly smoketested; am posting it for the sake of
discussion.
I don't think this API will make the non-C++-fans happier; I think the
objection to the work I just merged is that it's adding more C++ than
those people are comfortable with.
So although the attached patch makes things shorter (good), it's taking
things in a "more C++" direction (questionable). I'd hoped to paper
over the C++ somewhat.
I suspect that any API which requires the of < > characters within the
implementation of a gimple pass to mean a template is going to give
those less happy with C++ a visceral "ugh" reaction. I wonder if
there's a way to spell these things that's concise and which doesn't
involve <> ?
wasnt that my last thought? is_a_call(), as_a_call() and
dyn_cast_call () ?
I think lack of <> in identifiers helps us old brains parse faster
:-) <> are like ()... many many years of causing a certain kind of
break in mental processing. I'm accustomed to single <> these days, but
once you get into multiple <>'s I quickly loose track. I find the same
thing with ()... hence I'm not a lisp fan :-)
I dont think 'operator *' c++ifies it too much, but I still think
operator gimple() would be easier... no extra character at all, and no
odd looking dereference of a non-pointer object or double dereference of
a pointer. I cant think of how that could get us into trouble... it'll
always map to the stmt the iterator currently points to.
Andrew
