Re: -Wformat-overflow handling for %b and %B directives in C2X standard

[Marek Polacek <polacek@redhat.com>]

On Mon, Aug 15, 2022 at 07:42:39PM +0300, Frolov Daniil wrote:
> вт, 12 апр. 2022 г. в 00:56, Marek Polacek <polacek@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