"gimple-classes-v2-option-3" git branch committed to svn trunk as r217787

Andrew MacLeod amacleod@redhat.com
Thu Nov 20 14:22:00 GMT 2014


On 11/20/2014 08:08 AM, Richard Biener wrote:
> On Thu, Nov 20, 2014 at 12:05 AM, Andrew MacLeod <amacleod@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
>>>> <amacleod@redhat.com> wrote:
>>>>> On 11/19/2014 03:43 PM, Richard Biener wrote:
>>>>>> On November 19, 2014 8:26:23 PM CET, Andrew MacLeod
>>>>> <amacleod@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.
>
Fine by me, Just running through the options to make sure we know what 
we are getting :-)

Andrew



More information about the Gcc-patches mailing list