<div>(switching back to cfe-commits, this isn't an llvm patch...)</div><div><br></div><div>This patch is pretty good. I just want to nit pick a bit w/ the way you're using FileCheck. It won't cause you problems today, but it may save you some painful debugging tomorrow to get into this habit.</div>
<div><br></div><div>One the tricky things about FileCheck is 'CHECK-NOT'. It often doesn't do what people think it does. In particular, it only checks that there is not a match starting from the previously matched position until the next matched position. Thus, in some cases, your test might silently pass even though there was a bug, because the first CHECK-NOT would scan the entire file, and the second CHECK-NOT would start from the end of the file, and go to the end of the file.</div>
<div><br></div><div>There are lots of little gotchas w/ CHECK-NOT. The way I try to use it is to always have explicit bounds:</div><div><br></div><div>CHECK: function_name</div><div>CHECK-NOT: bad_attribute</div><div>CHECK: ret</div>
<div><br></div><div>The idea is to pincer the -NOT predicate between two that we're certain will match. This is made easier because FileCheck is not actually operating on a line-by-line basis, so the above will detect the following buggy pattern:</div>
<div><br></div><div>def function_name bad_attribute {</div><div>  ...</div><div>  ret</div><div>}</div><div><br></div><div>(completely making up syntax here, but you get the point).</div><div><br></div><div>I'd try to use this to harden these filecheck tests a bit.</div>
<div><br></div><div><br></div><div>Also, just to make me happy about test coverage, want to add the attribute to another objc construct, and check that we then suppress the LLVM function attribute in that case?</div><div>
<br></div><div>Feel free to check in whenever, I or someone else can follow-up in post-commit review if needed. Thanks for putting up w/ all the comments here. =]</div>