[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