On Wed, Mar 10, 2010 at 8:05 AM, Kamil Dudka <kdudka@xxxxxxxxxx> wrote:
> On Wed March 10 2010 02:09:22 Pavel Roskin wrote:
>> I wonder is the quirk handling could be skipped for EXPR_SELECT, but I'm
>> fine with it if it's harmless.
>
> That's more likely question for sparse developers. I have no test-case which
> goes through the EXPR_SELECT path, so that I can't test it actually.
Hi, Sorry for the late reply. I did play with your patch yesterday.
I believe this is the code we are talking about:
+warn_for_enum_conversions(struct expression *expr, struct symbol *type)
+{
+ switch (expr->type) {
+ case EXPR_CONDITIONAL:
+ case EXPR_SELECT:
+ do_warn_for_enum_conversions(expr->cond_true
+ /* handle the "?:" quirk */
+ ?: expr->conditional,
+ type);
+ do_warn_for_enum_conversions(expr->cond_false, type);
+ break;
+
+ default:
+ do_warn_for_enum_conversions(expr, type);
+ }
+}
+
I feel that we shouldn't do special handling for EXPR_CONDITIONAL here.
We should just make the warn_for_enum_conversions more robust.
Special handing EXPR_CONDITIONAL here is just an one off thing.
What if there is a nested EXPR_CONDITIONAL inside the expr->cond_true?
See, your do_warn_for_enum_conversions() still need to handle
EXPR_CONDITIONAL any way. So I argue that we don't need that
special case for EXPR_CONDITIONAL here.
BTW, EXPR_SELECT only appear after expand stage. So you should not
see EXPR_SELECT in the evaluate stage.
Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
