This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.

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.

I think you are over-engineering this and the user-interface is
awful.

> (1) implies warnings for allocations in excess of the size.  For
> the alloca/VLA warnings it also means warnings for allocations
> that may be unbounded.  (This feels like a bit of a wart.)
>
> (2) implies warnings for allocations in excess of PTRDIFF_MAX
> only.  For the alloca/VLA warnings it also disables warnings
> for allocations that may be unbounded (also a bit of a wart)
>
> (3) isn't treated consistently by all options (yet) but for
> the alloca/VLA warnings it means no warnings.  Since
> the argument value is stored in signed HOST_WIDE_INT this
> range is strictly negative.
>
> Any value from (3) could in theory be made special and used
> instead of -1 or HOST_WIDE_INT_MAX as a placeholder for
> PTRDIFF_MAX.  But no matter what the choice is, it removes
> the value from the usable set in (3) (i.e., it doesn't have
> the expected effect of disabling the warning).
>
> I don't see the advantage of picking -2 over any other negative
> number.  As inelegant as the current choice of HOST_WIDE_INT_MAX
> may be, it seems less arbitrary and less intrusive than picking
> a random value from the negative range.
>
> Martin
>
> PS The handling of these ranges isn't consistent across all
> the options because they were each developed independently
> and without necessarily aiming for it.  I think making them
> more consistent would be nice as a followup patch.  I would
> expect consistency to be achievable more easily if baking
> special cases into the design is kept to a minimum.  It
> would also help to remove some existing special cases.
> E.g., by introducing a distinct option for the special case
> of diagnosing unbounded alloca/VLA allocations and removing
> it from -W{alloca,vla}-larger-than=.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]