<br><br>On Tuesday, February 19, 2013, Adrian Prantl  wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
On Feb 18, 2013, at 4:28 PM, David Blaikie <<a href="javascript:;" onclick="_e(event, 'cvml', 'dblaikie@gmail.com')">dblaikie@gmail.com</a>> wrote:<br>
> On Mon, Feb 18, 2013 at 4:24 PM, Adrian Prantl <<a href="javascript:;" onclick="_e(event, 'cvml', 'aprantl@apple.com')">aprantl@apple.com</a>> wrote:<br>
> On Feb 15, 2013, at 3:59 PM, Dmitri Gribenko <<a href="javascript:;" onclick="_e(event, 'cvml', 'gribozavr@gmail.com')">gribozavr@gmail.com</a>> wrote:<br>
> ><br>
> > +    ObjCInterfaceDecl* decl = cast<ObjCInterfaceType>(Ty)->getDecl();<br>
> > +    if (decl)<br>
> ><br>
> > cast<> can not return null (it will either succeed or assert in<br>
> > +Asserts mode, or just produce a wrong value in -Asserts).  Use<br>
> > dyn_cast (that returns null on failure) or drop the check -- whatever<br>
> > is appropriate.  And there's a dyn_cast idiom:<br>
> ><br>
> > if (Foo *F = dyn_cast<Foo>(Blah))<br>
> > ... use F...<br>
><br>
> I'm actually not checking the return value from cast<>() but the the result of getDecl() [which just happens to also return an ObjCInterfaceDecl]. Do you think I should use a temporary for the result of the cast to make it clearer?<br>

><br>
> Then you're missing test cases, because dyn_cast doesn't propagate null. It should fail if you pass it null.<br>
><br>
> You  probably want cast_or_null.<br>
<br>
At least in my particular case I'm not so sure about this. The code fragment we are talking about looks like this:<br>
<br>
 switch (Ty->getTypeClass()) {<br>
  ...<br>
  case Type::ObjCInterface: {<br>
    ObjCInterfaceDecl* Decl = cast<ObjCInterfaceType>(Ty)->getDecl();<br>
    if (Decl)<br>
       ...<br>
  }<br>
<br>
At this point Ty should always be nonzero and since we switch over the TypeClass the cast<> should always be safe, right?<br>
</blockquote><div><br></div><div>My initial comment was wrong -- sorry.  But you can still improve the code by moving the declaration into if:</div><div> </div><div><font><span style="line-height:normal;background-color:rgba(255,255,255,0)">if (ObjCInterfaceDecl* Decl = cast<ObjCInterfaceType>(Ty)->getDecl()) ...</span></font><br>
</div><div><font><span style="line-height:normal;background-color:rgba(255,255,255,0)"><br></span></font></div><div><font><span style="line-height:normal;background-color:rgba(255,255,255,0)">Dmitri<span></span></span></font></div>
<div><font><span style="line-height:normal;background-color:rgba(255,255,255,0)"><br></span></font></div><br><br>-- <br>main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if<br>(j){printf("%d\n",i);}}} /*Dmitri Gribenko <<a href="mailto:gribozavr@gmail.com" target="_blank">gribozavr@gmail.com</a>>*/<br>