<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Oct 21, 2014 at 8:11 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">----- Original Message -----<br>
> From: "Richard Smith" <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> To: <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>, <a href="mailto:dberlin@dberlin.org">dberlin@dberlin.org</a>, "aaron ballman" <<a href="mailto:aaron.ballman@gmail.com">aaron.ballman@gmail.com</a>>, <a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a><br>
> Cc: <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> Sent: Monday, October 20, 2014 4:17:00 PM<br>
> Subject: Re: [PATCH] Enhanced ignored-qualifiers checks for restrict<br>
><br>
> First off, more warnings for dubious uses of `restrict` seem valuable<br>
> to me. The two warnings you suggest seem like good ideas.<br>
<br>
</span>Thanks! There's more coming too...<br>
<br>
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.<br>
<span class=""><br>
> Some<br>
> thoughts:<br>
><br>
> I don't think cast expressions are the right thing to target here.<br>
> The same issue applies to compound literals, new-expressions, and so<br>
> on.<br>
<br>
</span>Good point.<br>
<span class=""><br>
> `restrict` only has meaning when used in "a declaration of an<br>
> ordinary identifier that provides a means of designating an object<br>
> P", so perhaps we should warn on it (at least at the top level) in<br>
> all cases where it's not part of such a declaration (and can't be<br>
> indirectly part of a declaration, such as if it's in a typedef or<br>
> template argument).<br>
<br>
</span>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.<br>
<br>
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?</blockquote><div><br></div><div>In SemaType's GetFullTypeForDeclarator, you have access to the Context from the DeclSpec, which you can use to determine whether to warn. (You'd probably want to add a check for whether the type from the decl-specifiers is 'restrict', and also check each time you add a pointer type whether that type introduces a 'restrict').</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> Taking the address of a `restrict`-qualified pointer has weird<br>
> semantics, and perhaps deserves a warning. For instance:<br>
><br>
> int *restrict a = &...;<br>
> int **p = &a;<br>
> *a = 1;<br>
> int k = **p;<br>
><br>
> ... is fine, even though `a` is accessed through a<br>
> non-`restrict`-qualified pointer, because `*q` is based on `a`.<br>
<br>
</span>This seems like a reasonable idea, I'll think about it. We probably want a warning like:<br>
<br>
pointer access '**p' is based on restrict-qualified pointer 'a' in a non-obvious way<br>
<span class=""><br>
><br>
> Similarly, declaring an object of a type that has a non-top-level<br>
> `restrict` qualifier also has weird semantics, and we should perhaps<br>
> warn on that.<br>
<br>
</span>Do you mean if someone tries to declare something like?<br>
int * * restrict * restrict * x;</blockquote><div><br></div><div>Sure. I was thinking of simpler things like</div><div><br></div><div>int *a = ...;</div><div>int **p = &a;</div><div>int *restrict *q = p;</div><div><br></div><div>// Now *p is based on 'a', which is 'restrict'ed because it can be designated as the restrict-qualified pointer *p. But accessing 'a' directly or through *p is fine, because those expressions are *also* based on 'a'. So the 'restrict' here perhaps doesn't mean what its author intended. Or maybe it does. *shrug*</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> Perhaps we should also warn on copying a `restrict`-qualified pointer<br>
> to a non-`restrict`-qualified pointer, since that also seems like a<br>
> very common error.<br>
<br>
</span>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.<br></blockquote><div><br></div><div>Consider this trivial example:</div><div><br></div><div> int *restrict a = malloc(sizeof(int) * 4);</div><div> for (int *p = a; p != a + 4; ++p)</div><div> *p = 0;</div><div> int result = a[n];</div><div><br></div><div>This has undefined behavior because p is not based on a, and p is used to modify an object accessed through a.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thanks again,<br>
Hal<br>
<br>
><br>
> <a href="http://reviews.llvm.org/D5872" target="_blank">http://reviews.llvm.org/D5872</a><br>
><br>
><br>
><br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div></div>