On 07/10/18 23:38, Martin Sebor wrote:
+ pretty_printer posval;
+ if (pos != error_mark_node)
+ {
+ /* Only format the position value when it's valid. By convention
+ do not quote constant integers. */
+ pp_space (&posval);
+ if (TREE_CODE (pos) != INTEGER_CST)
+ pp_begin_quote (&posval, pp_show_color (global_dc->printer));
+ dump_generic_node (&posval, pos, 0, TDF_NONE, false);
+ if (TREE_CODE (pos) != INTEGER_CST)
+ pp_end_quote (&posval, pp_show_color (global_dc->printer));
+ }
Sorry for the bike-shedding but is this really necessary?
First, you handle != INTEGER_CST separately, so you can simply use %qE for that
case and %E for the rest. Nevertheless, I think the convention is
(https://gcc.gnu.org/wiki/DiagnosticsGuidelines#Quoting): "elements such as
numbers that do no refer to numeric constants that appear in the source code
should not be quoted". Since this is a integer constant that appears in the
source code, then it should be quoted.
Also, "value%s" where %s can be empty, will not translate correctly.
Perhaps you need a small function:
warn_attributes(const char * msg_no_value_no_arg,
const char * msg_no_value_arg,
const char * msg_value_no_arg,
const char * msg_value_arg,
tree atname, int argno, tree pos, ...)
+ if (TREE_CODE (pos) != INTEGER_CST)
+ {
+ /* Only mention the argument number when it's non-zero. */
+ if (argno < 1)
+ warning (OPT_Wattributes,
+ "%qE attribute argument value%s is not an integer "
+ "constant",
+ atname, pp_formatted_text (&posval));
+ else
+ warning (OPT_Wattributes,
+ "%qE attribute argument %i value%s is not an integer "
+ "constant",
+ atname, argno, pp_formatted_text (&posval));
+
+ return NULL_TREE;
+ }
So that in the code above you can say:
if (TREE_CODE (pos) != INTEGER_CST)
{
warn_attributes("%qE attribute argument value is not an integer",
"%qE attribute argument %i value is not an integer",
"%qE attribute argument value %qE is not an integer",
"%qE attribute argument %i value %qE is not an integer",
atname, argno, pos);
return NULL_TREE;
}
Also, I wonder where input_location is pointing at for these warnings. There
may be a better location. Clang is doing:
<source>:5:1: error: 'alloc_align' attribute requires parameter 1 to be an
integer constant
ALIGN ("1") void*
^ ~~~
<source>:1:36: note: expanded from macro 'ALIGN'
#define ALIGN(N) __attribute__ ((alloc_align (N)))
^ ~
while GCC does:
<source>:6:16: warning: alloc_align parameter outside range [-Wattributes]
6 | fpvi_str_1 (int);
| ^
Apart from the above, this seems a major improvement, so I hope it goes in.
Cheers,
Manuel.
