On Mon, Aug 15, 2022 at 07:42:39PM +0300, Frolov Daniil wrote: > вт, 12 апр. 2022 г. в 00:56, Marek Polacek <pola...@redhat.com>: > > > > > On Thu, Apr 07, 2022 at 02:10:48AM +0500, Frolov Daniil wrote: > > > Hello! Thanks for your feedback. I've tried to take into account your > > > comments. New patch applied to the letter. > > > > Thanks. > > > > > The only thing I have not removed is the check_std_c2x () function. From > > > my > > > point of view -Wformat-overflow shouldn't be thrown if the standard < C2X. > > > So it's protection for false triggering. > > > > Sorry but I still think that is the wrong behavior. If you want to warn > > about C2X constructs in pre-C2X modes, use -Wpedantic. But if you want > > to use %b/%B as an extension in older dialects, that's OK too, so I don't > > know why users would want -Wformat-overflow disabled in that case. But > > perhaps other people disagree with me. > > > Hi! Sorry for the late reply. If we want to look at it as on extension > then I am agreed with you. > Removed this function in new patch.
Thanks, the patch looks good to me (I have one comment though), but I can't approve it. > @@ -1229,6 +1231,10 @@ format_integer (const directive &dir, tree arg, > pointer_query &ptr_qry) > case 'u': > base = 10; > break; > + case 'b': > + case 'B': > + base = 2; > + break; > case 'o': > base = 8; > break; > @@ -1348,13 +1354,12 @@ format_integer (const directive &dir, tree arg, > pointer_query &ptr_qry) > } > > res.range.unlikely = res.range.max; > + unsigned adj = (sign | maybebase) + (base == 2 || base == 16); We have this same line here and ... > /* Bump up the counters if WIDTH is greater than LEN. */ > - res.adjust_for_width_or_precision (dir.width, dirtype, base, > - (sign | maybebase) + (base == 16)); > + res.adjust_for_width_or_precision (dir.width, dirtype, base, adj); > /* Bump up the counters again if PRECision is greater still. */ > - res.adjust_for_width_or_precision (dir.prec, dirtype, base, > - (sign | maybebase) + (base == 16)); > + res.adjust_for_width_or_precision (dir.prec, dirtype, base, adj); > > return res; > } > @@ -1503,17 +1508,16 @@ format_integer (const directive &dir, tree arg, > pointer_query &ptr_qry) > if (res.range.min == 1) > res.range.likely += base == 8 ? 1 : 2; > else if (res.range.min == 2 > - && base == 16 > + && (base == 16 || base == 2) > && (dir.width[0] == 2 || dir.prec[0] == 2)) > ++res.range.likely; > } > } > > + unsigned adj = (sign | maybebase) + (base == 2 || base == 16); ... here, but sign, maybebase, and base couldn't have changed meanwhile. So can we compute 'adj' just once after we've determined the base and sign, and make it const? And I think that if 'maybebase' is never changed in the function, it ought to be made const as well. Thanks, Marek
