On Thu, Nov 20, 2014 at 12:05 AM, Andrew MacLeod <amacl...@redhat.com> wrote: > 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 think we want to have a consistent style across GCC even if seen as ugly to some people. Thus having (member) functions for conversion in some cases and as_a <> templates in others is bad. C++ was supposed to make grok GCC easier for newbies - this is exactly making it harder (not that I believe in this story at all...) > 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. I dislike conversion operators. Why is 'operator *' bad? It's exactly how iterators are supposed to work - after all the gsi stuff was modeled after STL iterators! So that's a definitive "no" from me to is_a_call () as_a_call () etc. Richard. > Andrew
