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

[Martin Sebor]

Richard, do you have any further comments or suggestions or is
the patch acceptable?

I realize it's not ideal but I don't see how to achieve the ideal
(understanding PTRDIFF_MAX) without deferring the processing of
these options until the back end has been initialized.  It would
still mean setting aside some special value, traversing options
again, looking for the Host_Wide_Int ones with the special value,
and replacing it with the value of PTRDIFF_MAX.  That doesn't seem
any cleaner than the current solution, just a lot more work (not
just to implement but for the compiler to go through at startup).

Joseph, can you think of anything better?

   https://gcc.gnu.org/ml/gcc-patches/2018-07/msg01455.html

On 07/26/2018 02:52 PM, Martin Sebor wrote:
> 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
>