On Mon, Jul 09, 2018 at 07:55:20PM +0200, Daniel Vetter wrote:
On Mon, Jul 9, 2018 at 6:12 PM, Mark Rutland mark.rutland@arm.com wrote:
On Mon, Jul 09, 2018 at 06:03:42PM +0200, Peter Zijlstra wrote:
On Mon, Jul 09, 2018 at 05:52:04PM +0200, Daniel Vetter wrote:
for_each_something(foo) if (foo->bla) call_bla(foo); else call_default(foo);
Totally contrived, but this complains. Liberally sprinkling {} also shuts up the compiler, but it's a bit confusing given that a plain for {;;} is totally fine. And it's confusing since at first glance the compiler complaining about nested if and ambigous else doesn't make sense since clearly there's only 1 if there.
Ah, so the pattern the compiler tries to warn about is:
if (foo) if (bar) /* stmts1 */ else /* stmts2 *Because it might not be immediately obvious with which if the else goes. Which is fair enough I suppose.
OK, ACK.
Just to bikeshed, there could be macros other than for_each_*() macros that will want to use this internally, so perhaps it would be worth the generic version being named something like if_noelse().
We could always add that as/when required, though.
I think a better name would be really good, but both when we added it for i915 and when we move it to drm headers we drew a blank. if_noelse() describes pretty good what it does, but kinda fails on the "where should I use it" departement. If there's some consensus I can sed the patches quickly.
Just to be clear: for_each_if() is fine by me, so no need to change things.
Sorry for the noise!
Mark.