[cfe-commits] [PATCH] llvm.annotation support

Julien Lerouge jlerouge at apple.com
Fri Sep 9 14:42:37 PDT 2011


On Fri, Sep 09, 2011 at 10:47:56AM -0700, John McCall wrote:
> On Sep 9, 2011, at 10:37 AM, Julien Lerouge wrote:
> > On Mon, Sep 05, 2011 at 01:15:39PM -0700, John McCall wrote:
> >> 
> >> +  case Builtin::BI__builtin_annotation: {
> >> +    llvm::Value *AnnVal = EmitScalarExpr(E->getArg(0));
> >> +    llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::annotation,
> >> +                                      AnnVal->getType());
> >> +
> >> +    // Get the annotation string, go through casts.
> >> +    const Expr *AnnotationStrExpr = E->getArg(1)->IgnoreParenCasts();
> >> +    llvm::StringRef Str = cast<StringLiteral>(AnnotationStrExpr)->getString();
> >> +    return RValue::get(EmitAnnotationCall(F, AnnVal, Str, E->getExprLoc()));
> >> +  }
> >> 
> >> 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.
> >> 
> >> And then you should make that true by adding the appropriate case to Sema::CheckBuiltinFunctionCall. :)
> > 
> > Ok, the extra code is below. Thanks, for rewieving this! Let me know if
> > there is anything wrong.
> 
> The logic looks fine.  Please make CheckBuiltinAnnotationString a static
> global function instead of a member function of Sema;  I know you were
> probably following the existing practice in that file, but it's honestly not
> a very good practice. :)

Agreed.

> Do you have commit access?  If not, just send me a complete patch
> and I'll commit for you.

I think I have access, thanks for all the reviews.

Julien

-- 
Julien Lerouge
PGP Key Id: 0xB1964A62
PGP Fingerprint: 392D 4BAD DB8B CE7F 4E5F FA3C 62DB 4AA7 B196 4A62
PGP Public Key from: keyserver.pgp.com



More information about the cfe-commits mailing list