Re: [PATCH] treat -Wxxx-larger-than=HWI_MAX special (PR 86631)

[Martin Sebor <msebor at gmail dot com>]

On 07/26/2018 08:58 AM, Martin Sebor wrote:
> On 07/26/2018 02:38 AM, Richard Biener wrote:
> > On Wed, Jul 25, 2018 at 5:54 PM Martin Sebor <msebor@gmail.com> wrote:
> > > On 07/25/2018 08:57 AM, Jakub Jelinek wrote:
> > > > On Wed, Jul 25, 2018 at 08:54:13AM -0600, Martin Sebor wrote:
> > > > > I don't mean for the special value to be used except internally
> > > > > for the defaults.  Otherwise, users wanting to override the default
> > > > > will choose a value other than it.  I'm happy to document it in
> > > > > the .opt file for internal users though.
> > > > >
> > > > > -1 has the documented effect of disabling the warnings altogether
> > > > > (-1 is SIZE_MAX) so while I agree that -1 looks better it doesn't
> > > > > work.  (It would need more significant changes.)
> > > >
> > > > The variable is signed, so -1 is not SIZE_MAX.  Even if -1 disables
> > > > it, you
> > > > could use e.g. -2 or other negative value for the other special case.
> > >
> > > The -Wxxx-larger-than=N distinguish three ranges of argument
> > > values (treated as unsigned):
> > >
> > >    1.  [0, HOST_WIDE_INT_MAX)
> > >    2.  HOST_WIDE_INT_MAX
> > >    3.  [HOST_WIDE_INT_MAX + 1, Infinity)
> >
> > But it doesn't make sense for those to be host dependent.
>
> It isn't when the values are handled by each warning.  That's
> also the point of this patch: to remove this (unintended)
> dependency.
>
> > I think numerical user input should be limited to [0, ptrdiff_max]
> > and cases (1) and (2) should be simply merged, I see no value
> > in distinguishing them.  -Wxxx-larger-than should be aliased
> > to [0, ptrdiff_max], case (3) is achieved by -Wno-xxx-larger-than.

To be clear: this is also close to what this patch does.

The only wrinkle is that we don't know the value of PTRDIFF_MAX
either at the time the option initial value is set in the .opt
file or when the option is processed when it's specified either
on the command line or as an alias in the .opt file (as all
-Wno-xxx-larger-than options are).  Case (2) above is only
used by the implementation as a placeholder for PTRDIFF_MAX.
It's not part of the interface -- it's an internal workaround
for lack of a better word.

(There is an additional wrinkle in the -Walloca-larger-than=
has two modes both of which are controlled by a single option:
(a) diagnose only allocations >= PTRDIFF_MAX (default), and
(b) diagnose allocations > limit plus also unbounded/unknown
allocations.  I think these modes should be split up and (b)
controlled by a separate option (say something like
-Walloca-may-be-unbounded).

> > I think you are over-engineering this and the user-interface is
> > awful.
>
> Thank you.
>
> I agree that what you describe would be the ideal solution.
> As I explained in the description of the patch, I did consider
> handling PTRDIFF_MAX but the target-dependent value is not
> available at the time the option argument is processed.  We
> don't even know yet what the target data model is.
>
> This is the best I came up with.  What do you suggest instead?
>
> Martin