<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Feb 26, 2013 at 9:25 AM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">If nobody says anything to the contrary I’m inclined to commit this patch later today.<br>
</blockquote><div><br></div><div style>Please don't do that btw, if you've asked and I've been reviewing it I'll get to it as fast as I can, but I get busy too.</div><div style><br></div><div style>That said, while I'm still worried about the caching, I'm not going to hold it up on those bounds.</div>
<div style><br></div><div style>Few code issues:</div><div style><br></div><div>+  if (ID->getImplementation() == 0) {</div><div>+    CompletedTypeCache.erase(QualTy.getAsOpaquePtr());</div><div>+  }</div><div style><br>
</div><div style>No braces here.</div><div style><br></div><div style> +  bool Maybe = false;</div><div>+  switch (Ty->getTypeClass()) {</div><div>+  case Type::ObjCObjectPointer:</div><div>+    return maybeIncompleteInterface(cast<ObjCObjectPointerType>(Ty)->getPointeeType());</div>
<div>+  case Type::ObjCInterface: {</div><div>+    ObjCInterfaceDecl* Decl = cast<ObjCInterfaceType>(Ty)->getDecl();</div><div>+    if (Decl)</div><div>+      Maybe = (Decl->getImplementation() == 0);</div><div>
+  }</div><div>+  default:</div><div>+    return Maybe;</div><div>+  }</div><div><br></div><div style>How about replacing Maybe with just return <value>? Also you've got an odd fallthrough there. Also the * goes on the variable not the type. Also go ahead and merge the variable inside the conditional and you can then get rid of the braces as well.</div>
<div style><br></div><div style>-eric</div></div></div></div>