Re: [PATCH 02/89] Introduce gimple_switch and use it in various places

[Richard Henderson <rth at redhat dot com>]

On 04/23/2014 12:56 PM, Jeff Law wrote:
> On 04/22/14 15:38, Richard Henderson wrote:
>> On 04/22/2014 10:13 AM, David Malcolm wrote:
>>> On Mon, 2014-04-21 at 18:45 -0400, Trevor Saunders wrote:
>>>>> --- a/gcc/tree-loop-distribution.c
>>>>> +++ b/gcc/tree-loop-distribution.c
>>>>> @@ -687,8 +687,9 @@ generate_loops_for_partition (struct loop *loop,
>>>>> partition_t partition,
>>>>>           }
>>>>>             else if (gimple_code (stmt) == GIMPLE_SWITCH)
>>>>>           {
>>>>> +          gimple_switch switch_stmt = stmt->as_a_gimple_switch ();
>>>>
>>>> maybe it would make more sense to do
>>>> else if (gimple_switch switch_stmt = stmt->dyn_cast_gimple_switch ())
>>>
>>> Thanks.  Yes, or indeed something like:
>>>
>>>    else if (gimple_switch switch_stmt = dyn_cast <gimple_switch> (stmt))
>>>
>>> (modulo the "pointerness" issues mentioned in
>>> http://gcc.gnu.org/ml/gcc-patches/2014-04/msg01334.html )
>>>
>>
>> I'm not keen on embedding assignments into conditionals like this, much less
>> embedding variable declarations as well.  I think David's original is perfect.
> Likewise, though I am less annoyed by such things than I was in the past. 
> Perhaps that's an artifact of actually liking that kind of style for 'for' loops.

I'll admit that with *just* the declaration and assignment in the if,
it's not that bad, if others are strongly in favor of not reproducing the test
vs the enum value.

Perhaps I'm just reacting to previous uses of assignments within conditionals
in gcc, which looked more like

  if (test1
      && test2
      && (x = foo, test3(x))
      && test4(x)
      && (y = bar(x), test5))

and which caused all sorts of trouble.  Especially when the if conditional
expanded to fill an entire 80x25 screen.


r~