<div><br></div><div>Good idea; will do.  </div><div><br></div><div>  -DeLesley</div><br><div class="gmail_quote">On Wed, Mar 21, 2012 at 5:20 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">LGTM; one trivial comment added to codereview.<div><br></div><div>I also have some suggestions for diagnostic wording, but they're unrelated to the patch itself:</div>
<div><table cellpadding="0" cellspacing="0" style="font-family:Arial,Helvetica,sans-serif;font-size:13px;padding-top:5px;padding-right:5px;padding-bottom:5px;padding-left:5px">
<tbody><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top"></td></tr><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">
def warn_attribute_argument_not_lockable : Warning<
</td></tr><tr style="background-color:rgb(229,236,249)"><td></td></tr><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">
  "%0 attribute requires arguments whose type is annotated "
</td></tr><tr style="background-color:rgb(229,236,249)"><td></td></tr><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">
</td></tr><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">  "with 'lockable' attribute">,
</td></tr><tr style="background-color:rgb(229,236,249)"><td></td></tr><tr><td style="vertical-align:top"><font><span style="font-family:monospace;white-space:pre-wrap;font-size:12px">  InGroup<ThreadSafety>, DefaultIgnore;</span>
<div style="text-align:left"><span style="font-family:arial,helvetica,sans-serif;white-space:pre-wrap"><br></span></div></font><font face="arial, helvetica, sans-serif"><div style="white-space:pre-wrap;text-align:left"><span style="font-family:arial;text-align:-webkit-auto;white-space:normal">It would be useful for this diagnostic to point at the problematic argument and mention its type: "'guarded_by' attribute argument type 'MyMutex' is not annotated with 'lockable' attribute"</span><br>

</div><div style="text-align:left"><span style="white-space:pre-wrap"><br></span></div></font></td></tr><tr style="background-color:rgb(229,236,249)"><td></td></tr><tr><td style="text-align:left;white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">

</td></tr><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">def warn_attribute_argument_not_class : Warning<
</td></tr><tr style="background-color:rgb(229,236,249)"><td></td></tr><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">
  "%0 attribute requires arguments that are class type or point to class type">,
</td></tr><tr style="background-color:rgb(229,236,249)"><td></td></tr><tr><td style="white-space:pre-wrap;font-family:monospace;font-size:12px;vertical-align:top">
  InGroup<ThreadSafety>, DefaultIgnore;   </td></tr></tbody></table><br>This would read more naturally as "requires arguments that are of class type or pointer to class type". It would be useful to include the type here too.</div>

<div><br><div class="gmail_quote"><div class="im">On Thu, Mar 15, 2012 at 9:26 AM, Delesley Hutchins <span dir="ltr"><<a href="mailto:delesley@google.com" target="_blank">delesley@google.com</a>></span> wrote:<br></div>
<div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This patch downgrades the requirement that mutex expressions must<br>
refer to lockable objects from an error message to a warning.  The<br>
rest of the thread safety analysis works fine even if the class is not<br>
lockable, so there is no reason to break the build just because a<br>
class is missing the LOCKABLE attribute.<br>
<br>
<a href="http://codereview.appspot.com/5820063/" target="_blank">http://codereview.appspot.com/5820063/</a><br>
<span><font color="#888888"><br>
  -DeLesley<br>
<br>
--<br>
DeLesley Hutchins | Software Engineer | <a href="mailto:delesley@google.com" target="_blank">delesley@google.com</a> | <a href="tel:505-206-0315" value="+15052060315" target="_blank">505-206-0315</a><br>
</font></span></blockquote></div></div></div><br></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br>DeLesley Hutchins | Software Engineer | <a href="mailto:delesley@google.com" target="_blank">delesley@google.com</a> | 505-206-0315<br><br>