<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jul 1, 2011, at 3:48 PM, Caitlin Sadowski wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span style="font-family:arial, sans-serif;font-size:13px;background-color:rgb(255, 255, 255)">Here is the first thread safety patch. Note that this patch depends on prior refactoring patches I sent out. If you want to review it, you can go to:<div>
<br></div><div><a href="http://codereview.appspot.com/4667054" style="color:rgb(0, 0, 204)" target="_blank">http://codereview.appspot.com/4667054</a></div></span></blockquote><div><br></div><div>Hi Caitlin,</div><div><br></div><div>A few questions:</div><div><br></div><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div><div style="font-family:'Times New Roman';font-size:medium;background-color:transparent;margin-top:0px;margin-left:0px;margin-bottom:0px;margin-right:0px"><span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-weight:bold;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Summary</span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"></span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">We would like to add a set of thread safety attributes to Clang. These attributes, based on a prior GCC implementation, will allow for checkable documentation of basic locking policies in multithreaded programs. </span><br></div></div></blockquote></div></blockquote><div><br></div><div>Are these in mainline GCC or just a branch? If they are in mainline GCC and have shipped in a release, then taking them into Clang makes sense for compatibility reasons. If they are a research project that is in development, then the bar to getting it into mainline clang is much higher, because adding attributes is tantamount to a language extension.</div><div><br></div><blockquote type="cite"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; "><div><div style="font-family:'Times New Roman';font-size:medium;background-color:transparent;margin-top:0px;margin-left:0px;margin-bottom:0px;margin-right:0px"><span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-weight:bold;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">Plan</span><span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"></span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"></span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">We are planning to re-implement the GCC thread safety attributes in Clang. We will submit a series of patches to the cfe-commits mailing list. Our current plan for this serie is as follows:</span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap"></span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">1) Basic parsing and semantic checking of the attributes which do not take arguments. In other words, checking whether the attribute is applied in the appropriate context (e.g. to a field, with no arguments).</span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">2) Basic parsing and semantic checking of the attributes which do take arguments, but without parsing or checking the arguments themselves. At this point, we will simply discard the tokens making up the arguments. </span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">3) Attribute argument parsing.</span><br>
<span style="font-size:11pt;font-family:Arial;color:rgb(0, 0, 0);background-color:transparent;font-style:normal;font-variant:normal;text-decoration:none;vertical-align:baseline;white-space:pre-wrap">4) Adding the thread safety analysis checks. We will teach the static analysis-based warnings layer to warn for violations of the annotations discussed on the links above. </span><br></div></div></blockquote></div></blockquote><div><br></div><div>This sounds like a very reasonable breakdown. Please start with #0 though, which is "gain consensus that this is a good thing to take into mainline" :)</div><div><br></div><div>I'm specifically concerned on a number of fronts here:</div><div><br></div><div>1. Are these annotations general enough to apply to many common lock-based APIs?</div><div>2. How do the annotations handle aliasing of lock objects?</div><div>3. How many bugs have been found in practice?</div><div>4. Have you considered doing this as a static analysis (where the bar is much lower to get things into mainline) instead of as a language change?</div><div><br></div><div>-Chris</div><div><br></div><div><br></div></div></body></html>