<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Aug 30, 2011, at 10:36 PM, Julien Lerouge wrote:</div><blockquote type="cite"><div>Here is the second revision of this patch. It should address all the<br>previous comments. Thanks for all the feedback!<br></div></blockquote><div><br></div>This is looking a lot better, thanks.<br><div><br></div></div><div><div><font class="Apple-style-span" face="'Andale Mono'">+++ b/lib/CodeGen/CGBuiltin.cpp</font></div><div><font class="Apple-style-span" face="'Andale Mono'">@@ -456,7 +456,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,</font></div><div><font class="Apple-style-span" face="'Andale Mono'"> }</font></div><div><font class="Apple-style-span" face="'Andale Mono'"> </font></div><div><font class="Apple-style-span" face="'Andale Mono'"> case Builtin::BI__builtin_isfinite: {</font></div><div><font class="Apple-style-span" face="'Andale Mono'">- // isfinite(x) --> x == x && fabs(x) != infinity; }</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ // isfinite(x) --> x == x && fabs(x) != infinity;</font></div></div><div><br></div><div>This is fine, but please commit it separately.</div><div><br></div><div><div><font class="Apple-style-span" face="'Andale Mono'">+ case Builtin::BI__builtin_annotation: {</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ llvm::Value *AnnVal = EmitScalarExpr(E->getArg(0));</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::annotation,</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ AnnVal->getType());</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ // Get the annotation string, go through casts.</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ const Expr *AnnotationStrExpr = E->getArg(1)->IgnoreParenCasts();</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ llvm::StringRef Str = cast<StringLiteral>(AnnotationStrExpr)->getString();</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ return RValue::get(EmitAnnotationCall(F, AnnVal, Str, E->getExprLoc()));</font></div><div><font class="Apple-style-span" face="'Andale Mono'">+ }</font></div></div><div><br></div><div>Please have this comment say something like "Sema requires this to be a non-wide string literal, potentially casted", so that it's clear why this cast<> will always succeed.</div><div><br></div><div>And then you should make that true by adding the appropriate case to Sema::CheckBuiltinFunctionCall. :)</div><div><br></div><div>John.</div></body></html>