On Sun, Aug 28, 2016 at 06:32:59PM +0530, Prathamesh Kulkarni wrote:
> On 26 August 2016 at 21:25, Jason Merrill <ja...@redhat.com> wrote:
> > On Fri, Aug 26, 2016 at 7:39 AM, Prathamesh Kulkarni
> > <prathamesh.kulka...@linaro.org> wrote:
> >> However with C++FE it appears TYPE_RESTRICT is not set for the
> >> parameters (buf and fmt)
> >> and hence the warning doesn't get emitted for C++.
> >> C FE sets TYPE_RESTRICT for them. I am not sure how to workaround this
> >> issue, and would be grateful for suggestions.
> >
> > In the C++ FE you can see TYPE_RESTRICT on the types of the
> > DECL_ARGUMENTS of the function.
> Thanks for the suggestions, I could indeed see TYPE_RESTRICT set on
> DECL_ARGUMENTS.
> The attached patch warns for both C and C++ with -Wrestrict, and I
> have put it under -Wall.
> I suppose we don't want to warn for null pointer args ?
> for eg:
> void f(int *restrict x, int *y);
>
> void foo(void)
> {
> f(NULL, NULL) ?
> }
>
> However I suppose warning for pointer constants other than zero is desirable ?
> I am not sure whether restrict is valid for obj-c/obj-c++, so I have
> just kept it to C and C++
> in the patch. Should the warning also be included for obj-c and obj-c++ FE's ?
> Boostrapped and tested on x86_64-unknown-linux-gnu.
> OK to commit ?
>
> Thanks,
> Prathamesh
> >
> > Jason
> 2016-08-28 Prathamesh Kulkarni <prathamesh.kulka...@linaro.org>
>
> PR c/35503
> * doc/invoke.texi: Document -Wrestrict.
> c-family/
> * c-common.c (warn_for_restrict): New function.
> * c-common.h (warn_for_restrict): Declare it.
> * c.opt: New option -Wrestrit.
Typo.
> c/
> * c-parser.c (c_parser_postfix_expression_after_primary): Call
> warn_for_restrict.
> cp/
> * parser.c (cp_parser_postfix_expression): Likewise.
This is a ChangeLog entry in another directory, so Likewise wouldn't work here.
> testsuite/
> * c-c++-common/pr35503.c: New test-case.
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 3feb910..ec8778f 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -13057,4 +13057,27 @@ diagnose_mismatched_attributes (tree olddecl, tree
> newdecl)
> return warned;
> }
>
> +/* Warn if an argument at position param_pos is passed to a
> + restrict-qualified param, and it aliases with another argument. */
> +
> +void
> +warn_for_restrict (location_t loc, unsigned param_pos, vec<tree, va_gc>
> *args)
> +{
> + tree arg = (*args)[param_pos];
> + if (operand_equal_p (arg, null_pointer_node, 0))
> + return;
> +
> + for (unsigned i = 0; i < args->length (); ++i)
Use FOR_EACH_VEC_ELT?
> + {
> + if (i == param_pos)
> + continue;
> +
> + tree current_arg = (*args)[i];
> + if (operand_equal_p (arg, current_arg, 0))
> + warning_at (loc, 0,
> + "passing argument %u to restrict qualified parameter
> aliases with "
I think this should be "restrict-qualified".
> + "argument %u", param_pos + 1, i + 1);
> + }
> +}
> +
> #include "gt-c-family-c-common.h"
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index bc22baa..0526ad5 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -920,6 +920,7 @@ extern void c_parse_final_cleanups (void);
>
> extern void warn_for_omitted_condop (location_t, tree);
> extern void warn_for_memset (location_t, tree, tree, int);
> +extern void warn_for_restrict (location_t, unsigned, vec<tree, va_gc> *);
>
> /* These macros provide convenient access to the various _STMT nodes. */
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index a5358ed..a029a86 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1012,6 +1012,11 @@ Wduplicate-decl-specifier
> C ObjC Var(warn_duplicate_decl_specifier) Warning LangEnabledBy(C ObjC,Wall)
> Warn when a declaration has duplicate const, volatile, restrict or _Atomic
> specifier.
>
> +Wrestrict
> +C C++ Var(warn_restrict) Warning LangEnabledBy(C C++,Wall)
> +Warn when an argument passed to a restrict-qualified parameter aliases with
> +another argument.
> +
> ansi
> C ObjC C++ ObjC++
> A synonym for -std=c89 (for C) or -std=c++98 (for C++).
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index fe0c95f..54e1e87 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -8369,6 +8369,17 @@ c_parser_postfix_expression_after_primary (c_parser
> *parser,
> warn_for_memset (expr_loc, arg0, arg2, literal_zero_mask);
> }
>
> + if (TREE_CODE (expr.value) == FUNCTION_DECL && warn_restrict)
> + {
> + unsigned param_pos = 0;
> + for (tree t = TYPE_ARG_TYPES (TREE_TYPE (expr.value)); t; t =
> TREE_CHAIN (t), param_pos++)
> + {
> + tree type = TREE_VALUE (t);
> + if (POINTER_TYPE_P (type) && TYPE_RESTRICT (type))
> + warn_for_restrict (expr_loc, param_pos, exprlist);
> + }
> + }
> +
Line too long, but I think I'd prefer something like
if (TREE_CODE (expr.value) == FUNCTION_DECL && warn_restrict)
{
unsigned int param_pos = 0;
function_args_iterator iter;
tree t;
FOREACH_FUNCTION_ARGS (TREE_TYPE (expr.value), t, iter)
{
if (POINTER_TYPE_P (t) && TYPE_RESTRICT (t))
warn_for_restrict (expr_loc, param_pos, exprlist);
param_pos++;
}
}
instead.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 1f04501..ac8aab5 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -261,7 +261,7 @@ Objective-C and Objective-C++ Dialects}.
> -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align -Wcast-qual @gol
> -Wchar-subscripts -Wclobbered -Wcomment -Wconditionally-supported @gol
> -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
> --Wdelete-incomplete @gol
> +-Wdelete-incomplete -Wrestrict @gol
I thought these were supposed to be in alphabetical order.
> -Wno-deprecated -Wno-deprecated-declarations -Wno-designated-init @gol
> -Wdisabled-optimization @gol
> -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol
> @@ -5274,6 +5274,12 @@ compilations.
> Warn when deleting a pointer to incomplete type, which may cause
> undefined behavior at runtime. This warning is enabled by default.
>
> +@item -Wrestrict @r{(C and C++ only)}
> +@opindex Wrestrict
> +@opindex Wno-restrict
> +Warn when an argument passed to a restrict-qualified parameter
> +aliases with another argument
Missing period.
> +
> @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
> @opindex Wuseless-cast
> @opindex Wno-useless-cast
> diff --git a/gcc/testsuite/c-c++-common/pr35503.c
> b/gcc/testsuite/c-c++-common/pr35503.c
> new file mode 100644
> index 0000000..fb12a71
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr35503.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wrestrict" } */
> +
> +int foo (const char *__restrict buf, const char *__restrict fmt, ...);
> +
> +void f(void)
> +{
> + char buf[100] = "hello";
> + foo (buf, "%s-%s", buf, "world"); /* { dg-warning "passing argument 1 to
> restrict qualified parameter aliases with argument 3" } */
> +}
I'm not sure if this test coverage is enough...
Marek
