On Wed, Apr 23, 2014 at 01:32:24PM -0700, Richard Henderson wrote:
> 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.oh, I totally see where your coming from, and tbh I think most of the hand rolled downcasting in C++ I've written doesn't do it with an assignment in the if. That said I think its nice you can limmit the scope of the variable and have fewer lines. Trev > > 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~ >
signature.asc
Description: Digital signature
