On Thu, Jul 26, 2018 at 10:52 PM Martin Sebor <mse...@gmail.com> 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 <mse...@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).
But then why not make that special value accessible and handle it as PTRDIFF_MAX when that is available (at users of the params)? That is, Index: gcc/calls.c =================================================================== --- gcc/calls.c (revision 262951) +++ gcc/calls.c (working copy) @@ -1222,9 +1222,12 @@ alloc_max_size (void) if (alloc_object_size_limit) return alloc_object_size_limit; - alloc_object_size_limit - = build_int_cst (size_type_node, warn_alloc_size_limit); + HOST_WIDE_INT limit = warn_alloc_size_limit; + if (limit == HOST_WIDE_INT_MAX) + limit = tree_to_shwi (TYPE_MAX_VALUE (ptrdiff_type_node)); + alloc_object_size_limit = build_int_cst (size_type_node, limit); + return alloc_object_size_limit; } use sth like if (warn_alloc_size_limit == -1) alloc_object_size_limit = fold_convert (size_type_node, TYPE_MAX_VALUE (ptrdiff_type_node)); else alloc_object_size_limit = size_int (warn_alloc_size_limit); ? Also removing the need to have > int params values. It's that HOST_WIDE_INT_MAX use that is problematic IMHO. Why not use -1? Richard. > 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 >