This is the mail archive of the 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] Add -Wabsolute-value

On Fri, 24 Aug 2018, Martin Jambor wrote:

> +/* Assuming we have encountered a call to a probably wrong kind of abs, issue a
> +   warning.  LOC is the location of the call, FNKIND is a string characterizing
> +   the class of the used abs function, FNDEC is the actual function declaration
> +   and ATYPE is type of the supplied actual argument.  */

For proper i18n, you have to use an enumeration here, not English string 

> +  warning_at (loc, OPT_Wabsolute_value,
> +	      "using %s absolute value function %qD when argument "
> +	      "is of %s type %qT", fnkind, fndecl, act, atype);

And then have all the possible combinations as complete sentences, in 
separate warning_at calls in appropriate switch statments or marked up 
with G_() if you put more than one in a single warning_at call using ?:, 
for translation purposes.  (Any cases that are impossible combinations for 
the warning - you don't want translators to have to produce useless 
translations where e.g. both argument and call are of the same kind and so 
the warning shouldn't occur - should use gcc_unreachable ().)

> +    case BUILT_IN_FABS:
> +    case BUILT_IN_FABSF:
> +    case BUILT_IN_FABSL:
> +      if (!SCALAR_FLOAT_TYPE_P (atype)
> +	  || DECIMAL_FLOAT_MODE_P (TYPE_MODE (atype)))

Should include the _FloatN / _FloatNx built-in functions here (CASE_FLT_FN 
together with CASE_FLT_FN_FLOATN_NX).

> +    case BUILT_IN_CABS:
> +    case BUILT_IN_CABSF:
> +    case BUILT_IN_CABSL:

And use CASE_FLT_FN here rather than hardcoding the list (but we don't yet 
have _FloatN / _FloatNx built-in variants of cabs).

> +  tree ftype = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl)));
> +  if (tree_fits_uhwi_p (TYPE_SIZE (atype))
> +      && tree_to_uhwi (TYPE_SIZE (atype)) > tree_to_uhwi (TYPE_SIZE (ftype)))
> +    warning_at (loc, OPT_Wabsolute_value,
> +		"absolute value function %qD given an argument of type %qT "
> +		"but has parameter of type %qT which may cause truncation "
> +		"of value ", fndecl, atype, ftype);

Should not have space at end of warning text.  I don't think TYPE_SIZE is 
the right thing to use in general; for example, on x86_64, you should warn 
for passing a _Float128 value to fabsl, but both long double and _Float128 
are 16-byte types (with only 10 bytes non-padding in long double).

Joseph S. Myers

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