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]

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


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



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