[PATCH] Enhanced ignored-qualifiers checks for restrict

[Hal Finkel]

----- Original Message -----
> From: "Richard Smith" <richard at metafoo.co.uk>
> To: hfinkel at anl.gov, dberlin at dberlin.org, "aaron ballman" <aaron.ballman at gmail.com>, richard at metafoo.co.uk
> Cc: cfe-commits at cs.uiuc.edu
> Sent: Monday, October 20, 2014 4:17:00 PM
> Subject: Re: [PATCH] Enhanced ignored-qualifiers checks for restrict
> 
> First off, more warnings for dubious uses of `restrict` seem valuable
> to me. The two warnings you suggest seem like good ideas.

Thanks! There's more coming too...

For the record, I'd like to work on enhancing our support for restrict (as well as the hopefully-better C++ version being developed, WG21 N4150 has the current wording in progress). I think that enhancing our warnings on restrict is a good first step.

> Some
> thoughts:
> 
> I don't think cast expressions are the right thing to target here.
> The same issue applies to compound literals, new-expressions, and so
> on.

Good point.

> `restrict` only has meaning when used in "a declaration of an
> ordinary identifier that provides a means of designating an object
> P", so perhaps we should warn on it (at least at the top level) in
> all cases where it's not part of such a declaration (and can't be
> indirectly part of a declaration, such as if it's in a typedef or
> template argument).

As a note, and personally I find the wording here a bit unclear, but if you look at the rational document (WG14 N334), it is explicit that the wording also is intended to allow restrict to be useful on fields of structures. The identifier can be the thing that provides access to the structure.

But, generally speaking, I agree that the list of useful places is much smaller than the list of useless places, and an exclusionary design for the warning might make more sense. Do you have a suggestion on how the warning should be implemented?

> 
> Taking the address of a `restrict`-qualified pointer has weird
> semantics, and perhaps deserves a warning. For instance:
> 
>   int *restrict a = &...;
>   int **p = &a;
>   *a = 1;
>   int k = **p;
> 
> ... is fine, even though `a` is accessed through a
> non-`restrict`-qualified pointer, because `*q` is based on `a`.

This seems like a reasonable idea, I'll think about it. We probably want a warning like:

  pointer access '**p' is based on restrict-qualified pointer 'a' in a non-obvious way

> 
> Similarly, declaring an object of a type that has a non-top-level
> `restrict` qualifier also has weird semantics, and we should perhaps
> warn on that.

Do you mean if someone tries to declare something like?
  int * * restrict * restrict * x;

> 
> Perhaps we should also warn on copying a `restrict`-qualified pointer
> to a non-`restrict`-qualified pointer, since that also seems like a
> very common error.

I'm not sure about this one. It will be noisy, because as you noted above, this is perfectly legal behavior (the non-restrict-qualified variable's value simply becomes "based on" the restrict-qualified pointer), and carries well-defined (if sometimes hard to deduce) semantics.

Thanks again,
Hal

> 
> http://reviews.llvm.org/D5872
> 
> 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory