On Sun, Mar 03, 2019 at 10:10:45AM +0100, Thomas Weißschuh wrote: > On Sun, 2019-03-03T00:20+0100, Luc Van Oostenryck wrote: > > It's fine, thanks. I've taken Linus' version, adapted my description, > > renamed you test and added another test. I'm proposing to push the > > patch here under. > > Thanks! Sounds good to me. > > (A few minor notes below). > > Thomas > > > From 83cfb92761f4602bc08aa4be6fd9834a3b98d5e3 Mon Sep 17 00:00:00 2001 > > > > During the expansion of shifts, the variable 'conservative' is used > > to inhibit any possible diagnostics (for example, because he needed > Typo: ^^ the Yes, thanks. > > information is if the expression is a constant or not). > > The explanation about 'conservative' would be nice as a comment in the source. > I misunderstood its meaning before. > (Probably because I looked at the place where it was used to actually change > behaviour) You're right. It's far from obvious. See the patch here under. > > > > -static void check_shift_count(struct expression *expr, struct expression *right) > > +static void warn_shift_count(struct expression *expr, struct symbol *ctype, long long count) > > The 'struct symbol *ctype' could be 'const', after adapting the signatures of > the functions called with it. > If you want I can send a patch after this went in. Yes, sure. Best regards, -- Luc >From 7fd3778e2d3a7b17aefea66819bf07feb7a257d3 Mon Sep 17 00:00:00 2001 From: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx> Date: Sun, 3 Mar 2019 10:59:40 +0100 Subject: [PATCH] expand: add explanation to 'conservative' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The variable 'conservative' is used to allow testing some characteristics of an expression while inhibiting any possible side-efects like issuing a warning or marking the expression as erroneous. But this role is not immedialtely apparent. So, add a comment to the variable declaration. Suggested-by: Thomas Weißschuh <thomas@xxxxxxxx> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@xxxxxxxxx> --- expand.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/expand.c b/expand.c index 95f9fda63..455d5baef 100644 --- a/expand.c +++ b/expand.c @@ -47,6 +47,11 @@ static int expand_expression(struct expression *); static int expand_statement(struct statement *); + +// If set, don't issue a warning on divide-by-0, invalid shift, ... +// and don't mark the expression as erroneous but leave it as-is. +// This allows testing some characteristics of the expression +// without creating any side-effects (e.g.: is_zero_constant()). static int conservative; static int expand_symbol_expression(struct expression *expr) -- 2.21.0
