On Sat, Jan 09, 2016 at 11:20:27PM +0100, Nicolai Stange wrote: > Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx> writes: > > > > Shouldn't the following be more explicit? > > flag = expr_set_flag_mask(0, ...); > > flag = expr_set_flag_mask(in_flag, ...); > > flag = expr_clear_flag_mask(in_flag, ...); > > Yes, I know, it would need to duplicate the expr->flags at almost all calls. > > Admittedly, this looks way better. > > I'll change that to > void expr_set_flag(unsigned *flag, ...); > and likewise for the clearing guy. > > > > > Couldn't we get rid of those two function by separating the exclusive "bits" > > from the "sets"? > > Something like: > > #define __EXPR_FLAG_INT_CONST (1 << 0) > > #define __EXPR_FLAG_FP_CONST (1 << 1) > > ... > > #define EXPR_FLAG_INT_CONST (__EXPR_FLAG_INT_CONST | > > __EXPR_FLAG_INT_CONST_EXPR | > > __EXPR_FLAG_ARITH_CONST) > > No, this won't work since the "implied" bit masks are in general different for > setting and clearing a flag. > > For example, "integer constant" (i.e. integer literal) implies "integer > constant expression", but "not a integer constant" does not imply "not a > integer constant expression". Yes, sure, but it could work with one set of such macro to add flags and another one to clear them. I think it would be more clear and would avoid the need to have the two helper above. Not that it is critical, though. Yours, Luc -- 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