Re: [PATCHv3] Add a warning for invalid function casts
On Thu, Dec 7, 2017 at 3:48 PM, Bernd Edlingerwrote: > On 12/06/17 23:35, Jason Merrill wrote: >> On Fri, Dec 1, 2017 at 7:42 AM, Bernd Edlinger >> wrote: >>> this version of the patch improves the heuristic check to take the >>> target hook into account, to handle cases correctly when both or only >>> one parameter is _not_ promoted to int. >> >> In looking at this, I discovered that the argument to >> promote_prototypes should be the function type, not the parameter >> type; the existing uses in the C++ front end were wrong. >> > > Bah, sorry. > > Yes, it looks like there are at least two more target hooks that change > the type promotion rules later-on: targetm.calls.promote_function_mode > and PROMOTE_MODE. > > In the ada FE we pass NULL_TREE to promote_prototypes which seems to > mean if the target wants type promotion in principle. So it returns > true if some function may promote, and false if no function promotes. > At least this is how the sh-target handles this parameter. > This is BTW the only target that uses the argument of this callback. > > So I would think for the purpose of this warning the following check > should be sufficient: > >if (INTEGRAL_TYPE_P (t1) >&& INTEGRAL_TYPE_P (t2) >&& TYPE_PRECISION (t1) == TYPE_PRECISION (t2) >&& (TYPE_UNSIGNED (t1) == TYPE_UNSIGNED (t2) >|| !targetm.calls.promote_prototypes (NULL_TREE) >|| TYPE_PRECISION (t1) >= TYPE_PRECISION (integer_type_node))) > > > What do you think? > Is the patch OK with this change? Yes. Jason
Re: [PATCHv3] Add a warning for invalid function casts
On 12/07/17 21:48, Bernd Edlinger wrote: > On 12/06/17 23:35, Jason Merrill wrote: >> On Fri, Dec 1, 2017 at 7:42 AM, Bernd Edlinger >>wrote: >>> this version of the patch improves the heuristic check to take the >>> target hook into account, to handle cases correctly when both or only >>> one parameter is _not_ promoted to int. >> >> In looking at this, I discovered that the argument to >> promote_prototypes should be the function type, not the parameter >> type; the existing uses in the C++ front end were wrong. >> > > Bah, sorry. > > Yes, it looks like there are at least two more target hooks that change > the type promotion rules later-on: targetm.calls.promote_function_mode > and PROMOTE_MODE. > > In the ada FE we pass NULL_TREE to promote_prototypes which seems to > mean if the target wants type promotion in principle. So it returns > true if some function may promote, and false if no function promotes. > At least this is how the sh-target handles this parameter. > This is BTW the only target that uses the argument of this callback. > > So I would think for the purpose of this warning the following check > should be sufficient: > > if (INTEGRAL_TYPE_P (t1) > && INTEGRAL_TYPE_P (t2) > && TYPE_PRECISION (t1) == TYPE_PRECISION (t2) > && (TYPE_UNSIGNED (t1) == TYPE_UNSIGNED (t2) > || !targetm.calls.promote_prototypes (NULL_TREE) > || TYPE_PRECISION (t1) >= TYPE_PRECISION (integer_type_node))) > > > What do you think? > Is the patch OK with this change? > So, is the simplified heuristic using only promote_prototypes (NULL) OK? Thanks Bernd.
Re: [PATCHv3] Add a warning for invalid function casts
On 12/06/17 23:35, Jason Merrill wrote: > On Fri, Dec 1, 2017 at 7:42 AM, Bernd Edlinger >wrote: >> this version of the patch improves the heuristic check to take the >> target hook into account, to handle cases correctly when both or only >> one parameter is _not_ promoted to int. > > In looking at this, I discovered that the argument to > promote_prototypes should be the function type, not the parameter > type; the existing uses in the C++ front end were wrong. > Bah, sorry. Yes, it looks like there are at least two more target hooks that change the type promotion rules later-on: targetm.calls.promote_function_mode and PROMOTE_MODE. In the ada FE we pass NULL_TREE to promote_prototypes which seems to mean if the target wants type promotion in principle. So it returns true if some function may promote, and false if no function promotes. At least this is how the sh-target handles this parameter. This is BTW the only target that uses the argument of this callback. So I would think for the purpose of this warning the following check should be sufficient: if (INTEGRAL_TYPE_P (t1) && INTEGRAL_TYPE_P (t2) && TYPE_PRECISION (t1) == TYPE_PRECISION (t2) && (TYPE_UNSIGNED (t1) == TYPE_UNSIGNED (t2) || !targetm.calls.promote_prototypes (NULL_TREE) || TYPE_PRECISION (t1) >= TYPE_PRECISION (integer_type_node))) What do you think? Is the patch OK with this change? Thanks Bernd.
Re: [PATCHv3] Add a warning for invalid function casts
On Fri, Dec 1, 2017 at 7:42 AM, Bernd Edlingerwrote: > this version of the patch improves the heuristic check to take the > target hook into account, to handle cases correctly when both or only > one parameter is _not_ promoted to int. In looking at this, I discovered that the argument to promote_prototypes should be the function type, not the parameter type; the existing uses in the C++ front end were wrong. Jason